Skip to content

GraphScope 2/3: (graph-learn on vineyard) use vineyard fragment as the underlying graph.#210

Merged
sighingnow merged 7 commits intoalibaba:masterfrom
sighingnow:ht/macos
Aug 3, 2022
Merged

GraphScope 2/3: (graph-learn on vineyard) use vineyard fragment as the underlying graph.#210
sighingnow merged 7 commits intoalibaba:masterfrom
sighingnow:ht/macos

Conversation

@sighingnow
Copy link
Collaborator

No description provided.

@sighingnow sighingnow marked this pull request as draft July 26, 2022 05:38
@sighingnow sighingnow force-pushed the ht/macos branch 2 times, most recently from 4dfd1de to 4efd33a Compare July 29, 2022 03:03
@sighingnow sighingnow marked this pull request as ready for review August 1, 2022 03:08
@sighingnow sighingnow changed the title GraphLearn on vineyard: use vineyard fragment as the underlying graph. GraphScope 2/3: (graph-learn on vineyard) use vineyard fragment as the underlying graph. Aug 1, 2022
Graph* CreateRemoteGraph();
Graph* CreateLocalGraph(const std::string& type,
std::string const &view_type,
std::string const &use_attrs);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to use const T& for keeping consistency.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

// if (stopped_.load()) {
// return Status::OK();
// }
stopped_.store(true);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The stopped_ flag is to record the channel stopped state that I can see, so it seems that we should move the setting of stopped_ after HandleStop

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved.

Signed-off-by: Tao He <sighingnow@gmail.com>
Signed-off-by: Tao He <sighingnow@gmail.com>
Signed-off-by: Tao He <sighingnow@gmail.com>
…n about edges.

Signed-off-by: Tao He <sighingnow@gmail.com>
Copy link
Collaborator

@Seventeen17 Seventeen17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Signed-off-by: Tao He <sighingnow@gmail.com>
Signed-off-by: Tao He <sighingnow@gmail.com>
@sighingnow sighingnow merged commit e052949 into alibaba:master Aug 3, 2022
@sighingnow sighingnow deleted the ht/macos branch August 3, 2022 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants