[runtime env] Unify checks for empty runtime env using helper function#22129
Merged
edoakes merged 3 commits intoray-project:masterfrom Feb 7, 2022
Merged
Conversation
edoakes
added
the
@author-action-required
Collaborator
|
ping for merge |
rkooo567
requested changes
Feb 5, 2022
src/ray/common/runtime_env_manager.h
Outdated
| // Return whether a string representation of a runtime env represents an empty | ||
| // runtime env. It could either be "" (from the default string value in protobuf), | ||
| // or "{}" (from serializing an empty Python dict or a JSON file.) | ||
| bool IsRuntimeEnvEmpty(const std::string &serialized_runtime_env); |
Contributor
There was a problem hiding this comment.
Can you move it to a common, not runtime_env_manager.h? I think runtime_env_manager.h should be higher layer than things like TaskSpec
Contributor
Author
Collaborator
There was a problem hiding this comment.
LGTM, not sure if sang had a specific place in mind
Contributor
There was a problem hiding this comment.
Yeah no preference, tho we can probably name the file just common.h (since it is already under runtime env folder)
architkulkarni
removed
the
@author-action-required
rkooo567
approved these changes
Feb 7, 2022
simonsays1980
pushed a commit
to simonsays1980/ray
that referenced
this pull request
Feb 27, 2022
ray-project#22129) Followup from ray-project#21788. Previously we had a lot of `serialized_runtime_env == "{}" || serialized_runtime_env == ""` scattered around the C++ code; this PR puts this in a helper function.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why are these changes needed?
Followup from #21788. Previously we had a lot of
serialized_runtime_env == "{}" || serialized_runtime_env == ""scattered around the C++ code; this PR puts this in a helper function.Related issue number
Checks
scripts/format.shto lint the changes in this PR.