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

handle unnamed lambdas for consistency #15400

Merged
merged 1 commit into from
Sep 17, 2024
Merged

handle unnamed lambdas for consistency #15400

merged 1 commit into from
Sep 17, 2024

Conversation

zzstoatzz
Copy link
Collaborator

@zzstoatzz zzstoatzz commented Sep 17, 2024

this PR reorders the validation to catch invalid fn name fallbacks before attempting to create the flow in the API, and renames a fn set to an unnamed lambda (which currently falls back to <lambda>) to unknown-lambda to handle this case


besides disallowing < and > in names of Flow objects in the API, the flow decorator just works with lambdas (though granted its not recommended) and IMO we should either handle this case or explicitly not support lambdas (which we do already use for fn in the tests)

before

» python -c "from prefect import flow; flow(log_prints=True)(lambda: print(42))()"
Traceback (most recent call last):
...
prefect.exceptions.PrefectHTTPStatusError: Client error '422 Unprocessable Entity' for url 'https://api.prefect.cloud/api/accounts/xxx/workspaces/xxx/flows/'
Response: {'exception_message': 'Invalid request received.', 'exception_detail': [{'loc': ['body', 'name'], 'msg': "Name '<lambda>' contains an invalid character. Must not contain any of: ['/', '%', '&', '>', '<'].", 'type': 'value_error.invalidname'}], 'request_body': {'name': '<lambda>', 'tags': []}}

after

» python -c "from prefect import flow; flow(log_prints=True)(lambda: print(42))()"
21:34:07.570 | INFO    | prefect.engine - Created flow run 'psychedelic-bullmastiff' for flow 'unknown-lambda'
21:34:07.861 | INFO    | Flow run 'psychedelic-bullmastiff' - 42
21:34:08.022 | INFO    | Flow run 'psychedelic-bullmastiff' - Finished in state Completed()

Copy link

codspeed-hq bot commented Sep 17, 2024

CodSpeed Performance Report

Merging #15400 will not alter performance

Comparing lambda (dc8de4a) with main (8f159b4)

Summary

✅ 3 untouched benchmarks

@zzstoatzz zzstoatzz added the fix A fix for a bug in an existing feature label Sep 17, 2024
@zzstoatzz zzstoatzz marked this pull request as ready for review September 17, 2024 02:49
Copy link
Member

@cicdw cicdw left a comment

Choose a reason for hiding this comment

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

makes sense to me, dynamicism is the name of the game

@zzstoatzz zzstoatzz merged commit d9e78ee into main Sep 17, 2024
47 checks passed
@zzstoatzz zzstoatzz deleted the lambda branch September 17, 2024 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix A fix for a bug in an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants