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

Changed TaskSource key computation to handle OSError("source not available") #15583

Merged
merged 2 commits into from
Oct 4, 2024

Conversation

kzvezdarov
Copy link
Contributor

@kzvezdarov kzvezdarov commented Oct 3, 2024

In some cases, inspect.getsource can raise OSError("source not available"), in addition to OSError("could not find source") (see: https://github.com/python/cpython/blob/3.12/Lib/inspect.py#L1088 and https://github.com/python/cpython/blob/3.12/Lib/inspect.py#L1096)

It seems appropriate to handle both instances of OSError with the same fallback. This commit simply expands the fallback handler to also check for source not available errors.

Closes #15384

Checklist

  • This pull request references any related issue by including "closes <link to issue>"
    • If no issue exists and your change is not a small fix, please create an issue first.
  • If this pull request adds new functionality, it includes unit tests that cover the changes
  • If this pull request removes docs files, it includes redirect settings in mint.json.
  • If this pull request adds functions or classes, it includes helpful docstrings.

In some cases, `inspect.getsource` can raise
`OSError("source not available")`, in addition to
`OSError("could not find source")` (see:
https://github.com/python/cpython/blob/3.12/Lib/inspect.py#L1088
https://github.com/python/cpython/blob/3.12/Lib/inspect.py#L1096)

It seems appropriate to handle both instances of
`OSError` with the same fallback. This commit
simply expands the fallback handler to also check
for `source not available` errors.
@github-actions github-actions bot added bug Something isn't working labels Oct 3, 2024
Copy link

codspeed-hq bot commented Oct 3, 2024

CodSpeed Performance Report

Merging #15583 will not alter performance

Comparing kzvezdarov:fix-for-issue-15384 (bdd505d) with main (f524dc8)

Summary

✅ 3 untouched benchmarks

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.

the check needs a fix - I added a suggestion for how to do it.

Thank you for identifying and contributing this @kzvezdarov !!

src/prefect/cache_policies.py Outdated Show resolved Hide resolved
Co-authored-by: Chris White <white.cdw@gmail.com>
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.

awesome thank you!

@cicdw
Copy link
Member

cicdw commented Oct 4, 2024

Just a heads up - this will go out in our next 3.x release which will most likely occur on Monday or Tuesday next week (10/7 or 10/8).

@cicdw cicdw merged commit a12aeaf into PrefectHQ:main Oct 4, 2024
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

prefect-ray Local Deployment Error OSError('source code not available')
2 participants