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

Fix naming conflict between validate argument and imported function #8265

Merged
merged 2 commits into from
Feb 15, 2023

Conversation

scolebrook
Copy link
Contributor

The validate argument of c7n.policy.load() conflicts with the import statement on line 37 importing c7n.schema.validate(). Subsequent if expressions that appear to test the argument are actually testing that the callable c7n.schema.validate() function exists. This prevents anything calling load() with validate=False from preventing validation as expected.

The fix here is to import as to rename the callable rather than rename the argument which might break other things calling the load() function.

This will only change the behavior of anything calling load() with validate=False so that validation does not occur as intended.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Feb 11, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: scolebrook / name: Stephen Colebrook (7cd36c0)

@scolebrook
Copy link
Contributor Author

Related to #5409

Copy link
Contributor

@johnsca johnsca left a comment

Choose a reason for hiding this comment

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

Good catch, and I agree that renaming the import is the better (least invasive) fix.

Copy link
Collaborator

@kapilt kapilt left a comment

Choose a reason for hiding this comment

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

thanks, lgtm

@kapilt kapilt merged commit 23de69a into cloud-custodian:main Feb 15, 2023
@scolebrook scolebrook deleted the fix_policy_naming_conflict branch February 22, 2023 14:51
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.

3 participants