Skip to content
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

provide metaClient #275

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Conversation

Nicole00
Copy link
Contributor

@Nicole00 Nicole00 commented May 18, 2023

add meta client to support the storage client.

  • there's requirement for cloud to support exporting NebulaGraph's data, need to provide storage client scan interface in go sdk.
  • When scan data from nebula storaged server, the client need all storaged hosts and some schema informations, such as tag/edge schema, space part number.

related issue:#276

@Nicole00 Nicole00 requested review from Aiee and MegaByte875 May 18, 2023 03:07
@Nicole00 Nicole00 force-pushed the meta_client branch 2 times, most recently from bb10ee7 to a0bf908 Compare May 18, 2023 05:38
@codecov-commenter
Copy link

Codecov Report

Patch coverage: 50.00% and project coverage change: -1.31 ⚠️

Comparison is base (fae48de) 62.12% compared to head (c0a7f93) 60.82%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #275      +/-   ##
==========================================
- Coverage   62.12%   60.82%   -1.31%     
==========================================
  Files           9       10       +1     
  Lines        2305     2583     +278     
==========================================
+ Hits         1432     1571     +139     
- Misses        734      843     +109     
- Partials      139      169      +30     
Impacted Files Coverage Δ
meta_client.go 50.00% <50.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Comment on lines +115 to +124
if resp.GetCode() == nebula.ErrorCode_SUCCEEDED {
var spaces []string
for _, name := range resp.GetSpaces() {
spaces = append(spaces, string(name.GetName()))
}
return spaces, nil
} else {
return nil, fmt.Errorf("GetSpaces failed, code:%s", resp.GetCode())
}
}

Choose a reason for hiding this comment

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

How about return error first to reduce netsting else statement?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about return error first to reduce netsting else statement?

good idea, will change it.

- --meta_server_addrs=metad0:45500,metad1:45500,metad2:45500
- --local_ip=metad0
- --ws_ip=metad0
- --meta_server_addrs=172.28.1.1:45500,172.28.1.2:45500,172.28.1.3:45500

Choose a reason for hiding this comment

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

why change theses addresses

Copy link
Contributor Author

Choose a reason for hiding this comment

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

why change theses addresses

Because only the meta leader can return correct response. For client, it does not know the leader address, so client random chose one meta server addrs, when this addr is not leader, then client get the real leader from response and re-connect and re-execute.
So we should expose all the meta server address, or it will throw error for connection refused

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.

None yet

3 participants