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

feat: add QueryContext to RegionRequestHeader #156

Conversation

Kelvinyu1117
Copy link
Contributor

@Kelvinyu1117 Kelvinyu1117 commented Apr 27, 2024

I hereby agree to the terms of the GreptimeDB CLA

What's changed and what's your intention?

This refers to the issue "Pass query context down to region server service in Datanode".

Checklist

  • I have written the necessary comments.
  • I have added the necessary unit tests and integration tests.

Refer to a related PR or issue link (optional)

GreptimeTeam/greptimedb#3752

@Kelvinyu1117 Kelvinyu1117 force-pushed the refactor/kelvin-add-query-context branch from 2426058 to 6c609c9 Compare April 27, 2024 09:51
@Kelvinyu1117 Kelvinyu1117 marked this pull request as draft April 27, 2024 09:59
@Kelvinyu1117 Kelvinyu1117 marked this pull request as ready for review April 27, 2024 09:59
@Kelvinyu1117
Copy link
Contributor Author

Can you guys help to review when you have time? @killme2008 @MichaelScofield @WenyXu

@Kelvinyu1117 Kelvinyu1117 force-pushed the refactor/kelvin-add-query-context branch from 6c609c9 to 24c703b Compare April 28, 2024 06:13
@MichaelScofield
Copy link
Collaborator

@Kelvinyu1117 I'm ok with the proto change. But we need to change the GreptimeDB's codes first before merging this PR.

@Kelvinyu1117
Copy link
Contributor Author

Kelvinyu1117 commented Apr 28, 2024

@Kelvinyu1117 I'm ok with the proto change. But we need to change the GreptimeDB's codes first before merging this PR.

Shouldn't the existing GreptimeDB point to the old revision?
So the merge should be ok?

I guess I just need to change the revision to the latest commit of this repo in my branch of GreptimeDB in order to utilize this proto file?

greptime-proto = { git = "https://github.com/GreptimeTeam/greptime-proto.git", rev = "2c14c6e22dfe957f40bb88dd01fb8530656de89b" }

@MichaelScofield
Copy link
Collaborator

@Kelvinyu1117 If another person wants to make changes to proto, he would normally fork the main branch. If proto is merged too soon, he will see some compile errors not related to his codes.

@Kelvinyu1117
Copy link
Contributor Author

@Kelvinyu1117 If another person wants to make changes to proto, he would normally fork the main branch. If proto is merged too soon, he will see some compile errors not related to his codes.

Understood, so should I change the URL to my fork in my GreptimeDB's branch at the moment?
What is your usual practice?

@MichaelScofield
Copy link
Collaborator

@Kelvinyu1117 yes

@Kelvinyu1117 Kelvinyu1117 force-pushed the refactor/kelvin-add-query-context branch 2 times, most recently from 24c703b to 522cdc1 Compare April 28, 2024 07:35
@Kelvinyu1117 Kelvinyu1117 force-pushed the refactor/kelvin-add-query-context branch from 522cdc1 to 91f2a42 Compare May 2, 2024 15:22
@MichaelScofield MichaelScofield merged commit a191eda into GreptimeTeam:main May 6, 2024
6 checks passed
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