-
Notifications
You must be signed in to change notification settings - Fork 267
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
GraphScope on 3/3 part 2: pick up the previous graphscope functions #238
Conversation
Signed-off-by: Tao He <sighingnow@gmail.com>
Signed-off-by: Tao He <sighingnow@gmail.com>
graphlearn/python/client.py
Outdated
# one server finish, start to connect next server | ||
if self._client_cache[next_index] is None: | ||
self._client_cache[next_index] = pywrap.rpc_client( | ||
self._own_servers[next_index], True, True |
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.
There are only two args for pywrap.rpc_client
Signed-off-by: Tao He <sighingnow@gmail.com>
Signed-off-by: Tao He <sighingnow@gmail.com>
Signed-off-by: Tao He <sighingnow@gmail.com>
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.
LGTM
graphlearn/python/gsl/dag_dataset.py
Outdated
self._current_index = (self._current_index + 1) % len(self._client) | ||
# initialize the dataset for the next client | ||
if self._dag_datasets[self._current_index] is None: | ||
status = self._client.run_dag(self._dag.dag_def, self._current_index != len(self._client) - 1) |
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.
Maybe we can register dag on all the servers this client owned at once, to avoid delay with warming up when moving and consuming samples on next server.
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.
Fixed.
Signed-off-by: Tao He <sighingnow@gmail.com>
This pull request picks up necessary routines to support using graphlearn in the context of graphscope (see also the original work in e56fc9c).
This pull request has done a lot of cleanup and simplification, the main changes there are:
Client
class to support connecting to multiple servers from one client