Change core worker C++ namespace to ray::core #17610
Merged
Conversation
This file contains 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
raulchen
force-pushed
the
ray_core_namespace
branch
from
e916fab to
2d3b17f
Compare
|
Also need to deal with such classes: |
this PR only handles core worker code to avoid a huge PR |
scv119
approved these changes
Aug 6, 2021
raulchen
force-pushed
the
ray_core_namespace
branch
from
a93d02b to
8173087
Compare
kfstorm
reviewed
Aug 6, 2021
| @@ -21,6 +21,8 @@ | |||
| namespace ray { | |||
| namespace api { | |||
|
|
|||
| using ray::core::WorkerType; | |||
There was a problem hiding this comment.
Should we avoid using in header files?
There was a problem hiding this comment.
This header is not exposed to users. So I think it's fine. we should avoid using in api headers.
src/ray/core_worker/core_worker.cc
Outdated
| @@ -26,20 +26,20 @@ | |||
| #include "ray/util/util.h" | |||
|
|
|||
| namespace ray { | |||
| namespace core { | |||
| namespace { | |||
| @@ -24,6 +24,9 @@ | |||
| #include "ray/common/ray_object.h" | |||
| #include "ray/core_worker/core_worker.h" | |||
|
|
|||
| using namespace ray; | |||
| using namespace ray::core; | |||
|
Since we already have |
|
@kfstorm I tried changing everything, but that's too complex. I'd prefer keeping |
|
@raulchen Do you plan to rename |
|
we'll try changing other parts after releasing C++ worker. |
sven1977
pushed a commit
that referenced
this pull request
Aug 11, 2021
clarkzinzow
pushed a commit
to clarkzinzow/ray
that referenced
this pull request
Aug 12, 2021
clarkzinzow
added a commit
that referenced
this pull request
Aug 12, 2021
* [Core] Support ConcurrentGroup part1 (#16795) * Core change and Java change. * Fix void call. * Address comments and fix cases. * Fix asyncio * Change core worker C++ namespace to ray::core (#17610) * [C++ Worker] Replace `Ray::xxx` with `ray::xxx` and update namespaces (#17388) * [core] make 'PopWorker' to be an async function (#17202) * make 'PopWorker' to be an async function * pop worker async works * fix * address comments * bugfix * fix cluster_task_manager_test * fix * bugfix of detached actor * address comments * fix * address comments * fix aioredis * Revert "fix aioredis" This reverts commit 041b983. * bug fix * fix * fix test_step_resources test * format * add unit test * fix * add test case PopWorkerStatus * address commit * fix lint * address comments * add python test * address comments * make an independent function * Update test_basic_3.py Co-authored-by: Hao Chen <chenh1024@gmail.com> Co-authored-by: Qing Wang <kingchin1218@gmail.com> Co-authored-by: Hao Chen <chenh1024@gmail.com> Co-authored-by: qicosmos <383121719@qq.com> Co-authored-by: SongGuyang <guyang.sgy@antfin.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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?
In order to avoid namespace conflicts between ray core code and C++ worker code, we wanted to change the namespace of all ray core code to "ray::core". However, this turned out to be too complex.
Thus, this PR only changes core worker namespace, which should resolve most conflicts with the C++ worker.
Related issue number
#17388
Checks
scripts/format.shto lint the changes in this PR.