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

Add option to add arguments to PSRP hook and operator #27689

Merged
merged 4 commits into from
Jan 3, 2023

Conversation

malthe
Copy link
Contributor

@malthe malthe commented Nov 15, 2022

This adds an optional arguments parameter to the hook and operator which allows invoking cmdlets with arguments (rather than parameters, i.e. named arguments).

At the same time, passing parameters to invoke_cmdlet using **kwargs notation is deprecated; instead, parameters are now passed using the parameters optional keyword arguments.


Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

Comment on lines +231 to +237
warn(
"Passing **kwargs to 'invoke_cmdlet' is deprecated "
"and will be removed in a future release. Please use 'parameters' "
"instead.",
DeprecationWarning,
stacklevel=2,
)
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if it is worthwhile to do this rather than just bump the major version and break compatibility.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess it's nice enough to have a deprecation; doesn't cost much.

@malthe malthe force-pushed the add-arguments-to-psrp-operator branch from ab525c2 to 4cf3d10 Compare November 16, 2022 07:08
@github-actions
Copy link

github-actions bot commented Jan 1, 2023

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale Stale PRs per the .github/workflows/stale.yml policy file label Jan 1, 2023
malthe and others added 3 commits January 1, 2023 12:27
This also adds 'parameters' as an optional keyword argument to
'invoke_cmdlet', preferred over passing **kwargs (now deprecated).

The change to 'parameters' is motivated by the fact that most
parameters to cmdlets in Windows are capitalized, which is at odds
with the common Python parameter style.
Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>
@malthe malthe force-pushed the add-arguments-to-psrp-operator branch from 6906164 to ee84ad3 Compare January 1, 2023 11:28
@eladkal eladkal removed the stale Stale PRs per the .github/workflows/stale.yml policy file label Jan 1, 2023
@malthe
Copy link
Contributor Author

malthe commented Jan 2, 2023

@uranusjr should we consider merging this one? I rebased it.

Comment on lines +59 to 65
:param arguments:
When using the `cmdlet` or `powershell` option, use `arguments` to
provide arguments (templated).
:param parameters:
When using the `cmdlet` or `powershell` arguments, use this parameter to
When using the `cmdlet` or `powershell` option, use `parameters` to
provide parameters (templated). Note that a parameter with a value of `None`
becomes an *argument* (i.e., switch).
Copy link
Member

Choose a reason for hiding this comment

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

I’m assuming users fluent with Powershell would understand the difference between these? I wonder if we should add a link to some documentation about this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's a link in the provider documentation to https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-psrp/.

Most Windows superusers will be familiar with the Invoke-Command cmdlet which has the same interface.

@malthe malthe merged commit 3b6ced6 into apache:main Jan 3, 2023
@malthe malthe deleted the add-arguments-to-psrp-operator branch January 3, 2023 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants