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

Repair check of logbindir #1348

Merged
merged 2 commits into from
Jan 4, 2021
Merged

Repair check of logbindir #1348

merged 2 commits into from
Jan 4, 2021

Conversation

qha
Copy link
Contributor

@qha qha commented Dec 4, 2020

Presumably the intention was to check that it is both not relative
and not already managed.

After update to 10.8 the mysql module tries and fails to manage '.'
on nodes where i have log-bin set to a relative path.

@qha qha requested a review from a team as a code owner December 4, 2020 14:46
@puppet-community-rangefinder
Copy link

mysql::server::managed_dirs is a class

that may have no external impact to Forge modules.

This module is declared in 143 of 575 indexed public 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.

@gnif
Copy link

gnif commented Jan 1, 2021

I can confirm this issue is valid and the fix is correct. I just hit the identical issue.

@qha
Copy link
Contributor Author

qha commented Jan 2, 2021

I note that checks fail, i would be willing to put some time into repairing them if someone would provide some starting pointers.

@david22swan
Copy link
Member

Closing and re-opening pr in order to force the rekick of the tests. Some of the failures look like they may come from travis

@david22swan david22swan closed this Jan 4, 2021
@david22swan david22swan reopened this Jan 4, 2021
@puppet-community-rangefinder
Copy link

mysql::server::managed_dirs is a class

that may have no external impact to Forge modules.

This module is declared in 143 of 576 indexed public 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.

@david22swan
Copy link
Member

@qha Taking a quick look at the failures, quite a few seem to come from some missing changes.
Could you rebase your changes onto the main, that should clear some of them up and make diagnosis easier.

@david22swan david22swan self-assigned this Jan 4, 2021
Presumably the intention was to check that it is *both* not relative
and not already managed.

After update to 10.8 the mysql module tries and fails to manage '.'
on nodes where i have log-bin set to a relative path.
@qha
Copy link
Contributor Author

qha commented Jan 4, 2021

Rebased, hope that does the trick. Thank you for your time and advice! @david22swan

...even if $managed_dirs is empty.
@qha
Copy link
Contributor Author

qha commented Jan 4, 2021

Finally figured out at least one reason the tests were failing that was actually a bug (i don't think i introduced it, just changed things up so the tests caught it).

The remaining errors seem to be test environment problems if i understand the output...

@david22swan
Copy link
Member

@qha Thanks for responding so fast with the changes 👍
I did a few re-kicks of the tests and their all passing now so I'm happy to merge

@david22swan david22swan merged commit af4e7e5 into puppetlabs:main Jan 4, 2021
@qha
Copy link
Contributor Author

qha commented Jan 5, 2021

Thanks for merging! Do you know when a new release might happen? @david22swan

@qha
Copy link
Contributor Author

qha commented Jan 7, 2021

I see that a new version has been released now.

@pmcmaw
Copy link
Contributor

pmcmaw commented Jan 7, 2021

Yes apologies, this module has been released as of today and is now available on the Forge!
Thank you for your patience :-) @qha

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants