-
Notifications
You must be signed in to change notification settings - Fork 245
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
Added support for SkipHandlers #3802
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution, @v15h9. Requested changes/queries are marked with ±
@@ -62,6 +64,7 @@ class PackageInstallOptions(CCIModel): | |||
security_type: SecurityType = SecurityType.FULL | |||
apex_compile_type: Optional[ApexCompileType] = None | |||
upgrade_type: Optional[UpgradeType] = None | |||
skip_handlers: SkipHandlers = SkipHandlers.FEATURE_ENFORCEMENT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
±: We don't want to silently change behavior, so it shouldn't be a default. Instead, it should be an optional Option
set to None
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to None
if "skip_handlers" in task_options: | ||
options.skip_handlers = SkipHandlers(task_options["skip_handlers"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💭: Implementation note: The sf
flag allows multiple values and combines them into a comma-separated string for PackageInstallCreateRequest
. We don't need to do this here now, but we might need to if they add more SkipHandlers
in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a comment for future consideration.
@jstvz Any more changes required for this PR? |
SkipHandlers
added inPackageInstallRequest
starting from API Version 61.0 (Summer '24)