Add pulpcore 3.21 support #266
Conversation
There was a problem hiding this comment.
Looks good, but if you look at the failures (most visible in annotations @ https://github.com/theforeman/puppet-pulpcore/pull/266/files) you'll see that this issue has been resolved and now properly returns HTTP 200 again:
puppet-pulpcore/spec/support/acceptance/examples.rb
Lines 58 to 59 in 87aa4c4
I'd say that the easiest way to check it is ENV['BEAKER_FACTER_PULPCORE_VERSION'].
spec/support/acceptance/examples.rb
Outdated
| @@ -56,7 +56,11 @@ | |||
|
|
|||
| describe curl_command("https://#{host_inventory['fqdn']}/pulp/api/v3/", cacert: "#{certdir}/ca-cert.pem") do | |||
| # Requires authentication: https://github.com/pulp/pulpcore/issues/2340 | |||
| its(:response_code) { is_expected.to eq(403) } | |||
| if(ENV['BEAKER_FACTER_PULPCORE_VERSION'] == '3.21') | |||
There was a problem hiding this comment.
I'd prefer to be forward compatible and check if it isn't 3.17/3.18. Also because when you run it outside of CI (where we don't set the variable) it also ends up as 3.21 because that's the default in pulpcore::repo.
|
Tangential comment -- I took a look at Pulp changelogs to see what shiny new features we get after your work here is done. In particular, some users will definitely want to disable pulp/pulpcore#3115 . I took at a stab at that with #267 pulp/pulpcore#2329 was added previously in 3.19 and also looks like something that users may want to configure somewhat persistently for certain medium or longer term troubleshooting or monitoring scenarios, such as gathering data about intermittent or hard to reproduce issues. |
spec/support/acceptance/examples.rb
Outdated
| @@ -56,7 +56,11 @@ | |||
|
|
|||
| describe curl_command("https://#{host_inventory['fqdn']}/pulp/api/v3/", cacert: "#{certdir}/ca-cert.pem") do | |||
| # Requires authentication: https://github.com/pulp/pulpcore/issues/2340 | |||
| its(:response_code) { is_expected.to eq(403) } | |||
| if(ENV['BEAKER_FACTER_PULPCORE_VERSION'] !~ /3.1+/) | |||
There was a problem hiding this comment.
The + is kind of redundant since it means match 3.1 but also 3.1111. It's a partial match so it ends up working. A stricter match is:
| if(ENV['BEAKER_FACTER_PULPCORE_VERSION'] !~ /3.1+/) | |
| if(ENV['BEAKER_FACTER_PULPCORE_VERSION'] !~ /^3.1[6-9]$/) |
There was a problem hiding this comment.
Or you use the good old ['3.16', '3.17', '3.18', '3.19'].include?(…) ;)
(I don't mind either way, just don't like regular expressions too much 🙃)
There was a problem hiding this comment.
The
+is kind of redundant since it means match3.1but also3.1111. It's a partial match so it ends up working. A stricter match is:
I love how this is a bug someone would have to find when pulpcore 3.100 is out.😄
I think I like ['3.16', '3.17', '3.18', '3.19'].include?(…). Keeps it simple. Will update.
There was a problem hiding this comment.
The regex also would match 3116 ;)
|
Thanks @sjha4! |
Adding pulpcore 3.21 since we have that packaged. Working on other bits to enable this in katello and gems packaging.