Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(PE-27792) Fix version_is_less for current versioning scheme #1630

Merged

Conversation

nmburgan
Copy link

@nmburgan nmburgan commented Mar 10, 2020

The previous version of this function did not handle RC versions or post-release but not-RC-tagged versions vs. RC versions correctly (this is rare, but it crops up occasionally). This will allow us to compare dev builds to dev builds to determine which build is earlier.

@nmburgan nmburgan requested a review from highb March 10, 2020 19:47
@nmburgan nmburgan requested a review from a team as a code owner March 10, 2020 19:47
The previous version of this function did not handle RC versions or post-release but not-RC-tagged versions vs. RC versions correctly (this is rare, but it crops up occasionally). This will allow us to compare dev builds to dev builds to determine which build is earlier.
@nmburgan nmburgan force-pushed the issue/master/pe-27792_fix_version_is_less branch from 2acf139 to 2808970 Compare March 10, 2020 19:52
Copy link
Contributor

@genebean genebean left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@genebean genebean requested a review from gimmyxd March 10, 2020 20:33
@nmburgan
Copy link
Author

Not sure what's up with the acceptance test failure, but it doesn't seem related?


13:36:21 vice-microfilm.delivery.puppetlabs.net (windows2012r2-64-1) 20:36:21$ cmd.exe /c puppet apply --verbose --detailed-exitcodes C:/cygwin64/tmp/apply_manifest.pp.yhqKRW
13:36:21   Info: Loading facts
13:36:25   Notice: Compiled catalog for vice-microfilm.delivery.puppetlabs.net in environment production in 0.20 seconds
13:36:25   Info: Applying configuration version '1583872584'
13:36:25   Error: Error starting website: The object identifier does not represent a valid object. (Exception from HRESULT: 0x800710D8). Perhaps there is another website with this port or configuration setting
13:36:29   
13:36:29   Error: /Stage[main]/Main/Iis_site[1ba7a13edd7491a2d90a]/ensure: change from  to started failed: Error starting website: The object identifier does not represent a valid object. (Exception from HRESULT: 0x800710D8). Perhaps there is another website with this port or configuration setting
13:36:29   Notice: Applied catalog in 4.19 seconds
13:36:29 
13:36:29 vice-microfilm.delivery.puppetlabs.net (windows2012r2-64-1) executed in 7.82 seconds
13:36:29 Exited: 4
13:36:29         should run a second time without changes (FAILED - 1)
13:36:29       when puppet resource is run
13:36:29 
13:36:29 vice-microfilm.delivery.puppetlabs.net (windows2012r2-64-1) 20:36:29$ cmd.exe /c puppet resource iis_site 58893701a66848f05751
13:36:29   iis_site { '58893701a66848f05751':
13:36:34     ensure               => 'started',
13:36:34     applicationpool      => 'DefaultAppPool',
13:36:34     authenticationinfo   => {'anonymous' => 'true', 'basic' => 'false', 'clientCertificateMapping' => 'false', 'digest' => 'false', 'iisClientCertificateMapping' => 'false', 'windows' => 'false'},
13:36:34     bindings             => [{'bindinginformation' => '*:80:', 'protocol' => 'http'}],
13:36:34     enabledprotocols     => 'http',
13:36:34     limits               => {'connectiontimeout' => '120', 'maxbandwidth' => '4294967295', 'maxconnections' => '4294967295'},
13:36:34     logflags             => ['ClientIP', 'Date', 'HttpStatus', 'HttpSubStatus', 'Method', 'Referer', 'ServerIP', 'ServerPort', 'Time', 'TimeTaken', 'UriQuery', 'UriStem', 'UserAgent', 'UserName', 'Win32Status'],
13:36:34     logformat            => 'W3C',
13:36:34     loglocaltimerollover => 'false',
13:36:34     logpath              => '%SystemDrive%\inetpub\logs\LogFiles',
13:36:34     logperiod            => 'Daily',
13:36:34     logtruncatesize      => '20971520',
13:36:34     physicalpath         => 'C:\inetpub\wwwroot',
13:36:34     preloadenabled       => 'false',
13:36:34   }
13:36:34 
13:36:35 vice-microfilm.delivery.puppetlabs.net (windows2012r2-64-1) executed in 5.35 seconds
13:36:35 
13:36:35 vice-microfilm.delivery.puppetlabs.net (windows2012r2-64-1) 20:36:35$ cmd.exe /c puppet resource iis_site 1ba7a13edd7491a2d90a
13:36:35   iis_site { '1ba7a13edd7491a2d90a':
13:36:40     applicationpool      => 'DefaultAppPool',
13:36:40     authenticationinfo   => {'anonymous' => 'true', 'basic' => 'false', 'clientCertificateMapping' => 'false', 'digest' => 'false', 'iisClientCertificateMapping' => 'false', 'windows' => 'false'},
13:36:40     bindings             => [{'bindinginformation' => '*:8080:1ba7a13edd7491a2d90a', 'protocol' => 'http'}],
13:36:40     enabledprotocols     => 'http',
13:36:40     limits               => {'connectiontimeout' => '120', 'maxbandwidth' => '4294967295', 'maxconnections' => '4294967295'},
13:36:40     logflags             => ['ClientIP', 'Date', 'HttpStatus', 'HttpSubStatus', 'Method', 'Referer', 'ServerIP', 'ServerPort', 'Time', 'TimeTaken', 'UriQuery', 'UriStem', 'UserAgent', 'UserName', 'Win32Status'],
13:36:40     logformat            => 'W3C',
13:36:40     loglocaltimerollover => 'false',
13:36:40     logpath              => '%SystemDrive%\inetpub\logs\LogFiles',
13:36:40     logperiod            => 'Daily',
13:36:40     logtruncatesize      => '20971520',
13:36:40     physicalpath         => 'C:\inetpub\basic',
13:36:40     preloadenabled       => 'false',
13:36:40   }
13:36:40 
13:36:40 vice-microfilm.delivery.puppetlabs.net (windows2012r2-64-1) executed in 5.42 seconds
13:36:40         should run the first site on port 80
13:36:40         should run the second site on port 8080 (FAILED - 2)

Copy link
Contributor

@highb highb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acceptance tests passed so I think this is ready to merge if @gimmyxd thinks it looks good.

@genebean
Copy link
Contributor

If it’s just the IIS test and an the port related test that’s a known issue

Sent with GitHawk

@highb
Copy link
Contributor

highb commented Mar 10, 2020

@genebean That's what it looks like. Is that IIS test still useful or should it just be removed?

@gimmyxd
Copy link
Contributor

gimmyxd commented Mar 11, 2020

@genebean That's what it looks like. Is that IIS test still useful or should it just be removed?

@highb IIS has moved to litmus for a while, i think most of our modules did, we are testing a really old SHA of IIS in beaker pipeline and i'm not sure how relevant is this test now.

@mattkirby mattkirby merged commit 0cb045b into voxpupuli:master Mar 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants