Closed SimonHoenscheid closed 2 years ago
Puppetfiles
.These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.
Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.
Thank you for opening these PRs! I'll start taking a look at them today.
@m0dular I included the debian repo support feature here, because it contains variables used by this feature.
Ah that's right, they both use the repository related variables. I'll kick off the acceptance tests, and I'll find some time to add a couple unit tests to cover it. Unless you feel like doing those yourself :P
I always forget this, but for spec tests we need module deps in .fixtures.yml
. Could you add the apt
module to the forge_modules
key here: https://github.com/puppetlabs/influxdb/blob/eaa6d2dce3b972bbd1821564106d8279f763efbd/.fixtures.yml#L4-L7
One of the spec tests is failing because it expects the gpg url to be
https://repos.influxdata.com/influxdb2.key https://repos.influxdata.com/influxdb.key
But it only contains the second entry. I don't remember why I set it up that way, but I think we need the influxdb2.key
? I'm not 100% sure, but would it make sense to update the module data to have both urls?
@m0dular The repo descriptions just references one key:
I took a look at the gpg keys:
bastelfreak@bastelfreak-nb /tmp $ md5sum influxdb*
e7b6670153b90766ae2f5dc7bdc61c6a influxdb2.key
e38c47ef9138d841ee94f471f0ab01fb influxdb.key
e38c47ef9138d841ee94f471f0ab01fb influxdb.key.old
bastelfreak@bastelfreak-nb /tmp $
I didn't find an official statement from influxdb which key has which purpose, but:
The acceptance tests would show us whether removing the gpg key would be an issue, but I also think we should keep the GPG url the same for now. That can be revisited later if we need to.
@m0dular Only thing to think about: Yumrepo doesn't seem to have a Problem with two keys AFAIK I dont know if Apt has. so my suggestion would be to go with the old key for Ubuntu and Debian
Sounds good. If you can push a commit that fixes the $repo_url
error and adds the key back I'll kick off the acceptance tests. Should we have both keys in common.yaml
and add the old key to Debian.yaml
and Ubuntu.yaml
?
Sounds good. will do.
@m0dular PR updated :-)
I think the failures a related to the fact that the Debian an ubuntu repos for influx db only keep the latest version in them, we may need to specify latest or something for these OS's
https://repos.influxdata.com/ubuntu/dists/focal/stable/binary-arm64/Packages
I came across this before its really curious why they do this
@MartyEwings I will add that to the data
@MartyEwings @m0dular done. Do you want me to lint the code or should I rebase after #31 merged? I am also fine with closing #31 and linting #36 and #37
I went ahead and merged #31 because it's only linting. That created a merged conflict here on the closing )
of the parameters list for... some reason.
For #36 we could either apply the same $repo_url
and influxdb::repo_gpg_key_url
changes there and merge it, or just have those changes in this PR. Whichever way is easier works for me.
This PR is waiting for #36 to be merged. Afterwards this PR can be rebased.
I already rebased this PR. It can be merged after #36
I added a commit that includes the $package_require
code and an update to the spec tests, merged in #40. Thanks for the contribution!