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

Remove join from python_app decorator, and reimplement join_app #2621

Merged
merged 2 commits into from Mar 20, 2023

Conversation

benclifford
Copy link
Collaborator

@benclifford benclifford commented Mar 13, 2023

The join parameter should never be set by a user, and gives bad error messages if it is set - see issue #2147.

The parameter was only there so that the join_app decorator could be implemented in terms of the python_app decorator. This PR changes how that implementation works so that this misbehaviour is not exposed to end users.

Fixes issue #2147

Type of change

  • Bug fix (non-breaking change that fixes an issue)

The join parameter should never be set by a user, and gives bad error
messages if it is set - see issue #2147.

The parameter was only there so that the join_app decorator could be
implemented in terms of the python_app decorator. This PR changes how
that implementation works so that this misbehaviour is not exposed
to end users.

Fixes issue #2147
@benclifford benclifford merged commit 6c9cd66 into master Mar 20, 2023
@benclifford benclifford deleted the benc-joinapp-issue-2147 branch March 20, 2023 17:07
benclifford added a commit that referenced this pull request Aug 11, 2023
PR #2621 didn't remove the user facing join parameter it was intended to,
instead just ignoring it.

That PR #2621 also added a new executors parameter to the join_app decorator,
which is then ignored (as it should be - because join_apps should be forced
to run in the _parsl_internal executor). This PR removes that spurious
parameter.

This will be a breaking change for anyone explicitly specifying the
_parsl_internal decorator to join_app, or for anyone explicitly specifying
join=False to python_app. (Other uses of those parameters are already broken
because those parameters are ignored, and they will now be broken in
different ways)
khk-globus pushed a commit that referenced this pull request Aug 11, 2023
PR #2621 didn't remove the user facing join parameter it was intended to,
instead just ignoring it.

That PR #2621 also added a new executors parameter to the join_app decorator,
which is then ignored (as it should be - because join_apps should be forced
to run in the _parsl_internal executor). This PR removes that spurious
parameter.

This will be a breaking change for anyone explicitly specifying the
_parsl_internal decorator to join_app, or for anyone explicitly specifying
join=False to python_app. (Other uses of those parameters are already broken
because those parameters are ignored, and they will now be broken in
different ways)
khk-globus pushed a commit that referenced this pull request Aug 11, 2023
PR #2621 didn't remove the user facing join parameter it was intended to,
instead just ignoring it.

That PR #2621 also added a new executors parameter to the join_app decorator,
which is then ignored (as it should be - because join_apps should be forced
to run in the _parsl_internal executor). This PR removes that spurious
parameter.

This will be a breaking change for anyone explicitly specifying the
_parsl_internal decorator to join_app, or for anyone explicitly specifying
join=False to python_app. (Other uses of those parameters are already broken
because those parameters are ignored, and they will now be broken in
different ways)
khk-globus pushed a commit that referenced this pull request Aug 11, 2023
PR #2621 didn't remove the user facing join parameter it was intended to,
instead just ignoring it.

That PR #2621 also added a new executors parameter to the join_app decorator,
which is then ignored (as it should be - because join_apps should be forced
to run in the _parsl_internal executor). This PR removes that spurious
parameter.

This will be a breaking change for anyone explicitly specifying the
_parsl_internal decorator to join_app, or for anyone explicitly specifying
join=False to python_app. (Other uses of those parameters are already broken
because those parameters are ignored, and they will now be broken in
different ways)
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.

None yet

1 participant