-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
fix metadata storage path for RayTaskRunner
#16022
base: main
Are you sure you want to change the base?
Conversation
0da538f
to
dece978
Compare
CodSpeed Performance ReportMerging #16022 will not alter performanceComparing Summary
|
ecec785
to
7a41130
Compare
revert unintentional log removal yuhhhhhhhh
just install in the action whoops
6589666
to
4eb7b5c
Compare
if ( | ||
self.result_storage is None | ||
and update.get("result_storage") is None | ||
or (self.result_storage and self.result_storage.basepath is 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.
I think this change may have unintended consequences since it will now ignore all result storage that doesn't have a basepath, which is valid in cases like storing at the root of an S3 bucket.
I think we may need to dig deeper to find the root cause of this issue and why the block configuration isn't coming through correctly.
closes #16009
as described in the linked issue, we were writing result metadata files to the
cwd
when using a ray task runner because child processes would get aNone
storagebasepath
- this PR checks for this case and makes sure we also respect theresult_storage_key