Skip to content

feat: add QueryContext to RegionRequestHeader#156

Merged
MichaelScofield merged 1 commit intoGreptimeTeam:mainfrom
Kelvinyu1117:refactor/kelvin-add-query-context
May 6, 2024
Merged

feat: add QueryContext to RegionRequestHeader#156
MichaelScofield merged 1 commit intoGreptimeTeam:mainfrom
Kelvinyu1117:refactor/kelvin-add-query-context

Conversation

@Kelvinyu1117
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

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

Comment thread proto/greptime/v1/region/server.proto
@Kelvinyu1117 Kelvinyu1117 force-pushed the refactor/kelvin-add-query-context branch from 6c609c9 to 24c703b Compare April 28, 2024 06:13
@MichaelScofield
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Comment thread proto/greptime/v1/region/server.proto Outdated
@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
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