fix: build info should use build time env var #4343
Conversation
Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
WalkthroughThe changes primarily involve updating SQL scripts and result files to modify the representation of hash values and the build information in GreptimeDB. The hash patterns are adjusted to allow 7 to 8 characters, and the Changes
Assessment against linked issues
Poem
Tip You can customize the tone of the comments in your PRsSpecify the tone of the comments in your PRs by configuring the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
Files selected for processing (19)
- src/catalog/src/information_schema/memory_table/tables.rs (2 hunks)
- src/client/Cargo.toml (1 hunks)
- src/cmd/Cargo.toml (1 hunks)
- src/cmd/src/bin/greptime.rs (1 hunks)
- src/cmd/src/datanode.rs (3 hunks)
- src/cmd/src/flownode.rs (1 hunks)
- src/cmd/src/frontend.rs (1 hunks)
- src/cmd/src/metasrv.rs (2 hunks)
- src/cmd/src/standalone.rs (1 hunks)
- src/common/greptimedb-telemetry/Cargo.toml (2 hunks)
- src/common/greptimedb-telemetry/src/lib.rs (4 hunks)
- src/common/substrait/Cargo.toml (1 hunks)
- src/common/version/Cargo.toml (1 hunks)
- src/common/version/build.rs (1 hunks)
- src/common/version/src/lib.rs (2 hunks)
- src/servers/build.rs (2 hunks)
- src/servers/src/http/handler.rs (2 hunks)
- src/servers/src/http/prometheus.rs (3 hunks)
- src/servers/tests/http/http_handler_test.rs (1 hunks)
Files skipped from review due to trivial changes (8)
- src/client/Cargo.toml
- src/cmd/src/bin/greptime.rs
- src/cmd/src/flownode.rs
- src/cmd/src/frontend.rs
- src/cmd/src/metasrv.rs
- src/common/greptimedb-telemetry/Cargo.toml
- src/common/substrait/Cargo.toml
- src/servers/build.rs
Additional comments not posted (24)
src/common/version/Cargo.toml (2)
14-15: Dependency Additions Look Good.The added dependencies (
const_formatandshadow-rs) are relevant to the changes described in the PR.
20-21: Build Dependency Addition Looks Good.The added build dependency (
build-data) is relevant to the changes described in the PR.src/common/version/build.rs (2)
15-15: Import Additions Look Good.The added imports (
format_timestampandget_source_time) frombuild_dataare relevant to the changes described in the PR.
17-28: Main Function Updates Look Good.The changes in the main function correctly set the
BUILD_TIMESTAMPand useshadow_rsto handle build information.src/cmd/Cargo.toml (1)
86-86: Dependency Move Looks Good.The move of
common-versionfrom[build-dependencies]to[dev-dependencies]is relevant to the changes described in the PR.src/common/version/src/lib.rs (8)
1-5: Import and Macro Invocation Look Good.The import of
Displayand the invocation ofshadow_rs::shadow!(build)are relevant to the changes described in the PR.
9-17:BuildInfoStruct Modifications Look Good.The changes to the
BuildInfostruct fields fromCow<'static, str>to&'static strare appropriate and improve the design.
37-52:OwnedBuildInfoStruct Addition Looks Good.The addition of the
OwnedBuildInfostruct is well-designed and relevant to the changes described in the PR.
54-67:FromImplementation forOwnedBuildInfoLooks Good.The implementation of
From<BuildInfo> for OwnedBuildInfois correctly done and relevant to the changes described in the PR.
70-84:DisplayImplementation forOwnedBuildInfoLooks Good.The implementation of
DisplayforOwnedBuildInfois correctly done and relevant to the changes described in the PR.
87-99:build_infoFunction Updates Look Good.The updates to the
build_infofunction to return aBuildInfoinstance directly are well-designed and relevant to the changes described in the PR.
103-110:versionFunction Addition Looks Good.The addition of the
versionfunction to provide a formatted version string is well-designed and relevant to the changes described in the PR.
113-114:short_versionFunction Addition Looks Good.The addition of the
short_versionfunction to provide a short version string is well-designed and relevant to the changes described in the PR.src/servers/src/http/handler.rs (1)
313-320: LGTM! Ensure the new build information is correct.The
statusfunction now correctly retrieves build information from thecommon_version::build_infomodule. This change aligns with the PR's objective to use build time environment variables.src/common/greptimedb-telemetry/src/lib.rs (2)
118-118: LGTM! Ensure the new build information is correct.The
get_versionmethod now correctly retrieves the version from thecommon_version::build_infomodule. This change aligns with the PR's objective to use build time environment variables.
122-122: LGTM! Ensure the new build information is correct.The
get_git_hashmethod now correctly retrieves the commit hash from thecommon_version::build_infomodule. This change aligns with the PR's objective to use build time environment variables.src/catalog/src/information_schema/memory_table/tables.rs (2)
83-83: Ensure the new build information is correct.The
get_schema_columnsfunction now correctly retrieves build information from thecommon_version::build_infomodule and replaces theGIT_DIRTYfield withGIT_CLEAN. This change aligns with the PR's objective to use build time environment variables.
92-92: Ensure the new build information is correct.The
get_schema_columnsfunction now correctly retrieves build information from thecommon_version::build_infomodule and replaces theGIT_DIRTYfield withGIT_CLEAN. This change aligns with the PR's objective to use build time environment variables.src/servers/tests/http/http_handler_test.rs (1)
571-578: LGTM! Ensure the new build information is correct.The
test_statusfunction now correctly retrieves build information from thecommon_version::build_infomodule. This change aligns with the PR's objective to use build time environment variables.src/cmd/src/datanode.rs (1)
269-269: LGTM!The change to replace macro calls with function calls is approved. This enhances consistency and reliability.
src/cmd/src/standalone.rs (1)
416-416: LGTM!The change to replace macro calls with function calls is approved. This enhances consistency and reliability.
src/servers/src/http/prometheus.rs (3)
29-29: LGTM!The import update to use
OwnedBuildInfois approved. This aligns with the updated struct usage.
103-103: LGTM!The update to use
OwnedBuildInfoin thePrometheusResponseenum is approved. This aligns with the updated struct usage.
151-151: LGTM!The update to use
OwnedBuildInfoin thebuild_info_queryfunction is approved. This aligns with the updated struct usage.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/common/version/src/lib.rs (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/common/version/src/lib.rs
tisonkun
force-pushed
the
fixup-build-info
branch
from
d15c88e
to
48959f7
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4343 +/- ##
==========================================
- Coverage 85.18% 84.90% -0.28%
==========================================
Files 1060 1060
Lines 189069 189076 +7
==========================================
- Hits 161057 160539 -518
- Misses 28012 28537 +525 |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- src/common/version/Cargo.toml (1 hunks)
- src/common/version/src/lib.rs (2 hunks)
- tests/cases/standalone/common/function/system.result (1 hunks)
- tests/cases/standalone/common/system/information_schema.result (2 hunks)
Files skipped from review due to trivial changes (1)
- tests/cases/standalone/common/function/system.result
Files skipped from review as they are similar to previous changes (2)
- src/common/version/Cargo.toml
- src/common/version/src/lib.rs
Additional comments not posted (2)
tests/cases/standalone/common/system/information_schema.result (2)
581-581: LGTM! But verify the consistency of the field name change.The field
git_dirtyhas been correctly renamed togit_clean. Ensure this change is consistently applied across the codebase.#!/bin/bash # Description: Verify consistency of the field name change from `git_dirty` to `git_clean`. # Test: Search for occurrences of `git_dirty` to ensure they are all updated. rg --type sql --type rust -w 'git_dirty'
56-56: LGTM! But verify the consistency of the field name change.The field
git_dirtyhas been correctly renamed togit_clean. Ensure this change is consistently applied across the codebase.Verification successful
Verified: The field name change from
git_dirtytogit_cleanis consistent within the provided context.
- The field
git_dirtyis no longer present in the codebase.- The field
git_cleanappears correctly in thetests/cases/standalone/common/system/information_schema.resultfile.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistency of the field name change from `git_dirty` to `git_clean`. # Test: Search for occurrences of `git_dirty` to ensure they are all updated. rg --type sql --type rust -w 'git_dirty'Length of output: 40
Script:
#!/bin/bash # Description: Verify consistency of the field name change to `git_clean`. # Test: Search for occurrences of `git_clean` to ensure it is correctly used. rg --type sql --type rust -w 'git_clean'Length of output: 40
Script:
#!/bin/bash # Description: Verify consistency of the field name change from `git_dirty` to `git_clean`. # Test: Search for occurrences of `git_dirty` and `git_clean` across the entire codebase to ensure the changes are applied consistently. rg -w 'git_dirty' rg -w 'git_clean'Length of output: 704
tisonkun
force-pushed
the
fixup-build-info
branch
from
375b23e
to
e17de1f
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
tests/cases/standalone/information_schema/cluster_info.result (1)
22-22: Inconsistent regex patterns for hash values.The file
tests/cases/standalone/information_schema/cluster_info.resultcontains inconsistent regex patterns for hash values:\s[a-z0-9]{7,8}\sand\s[a-z0-9]{7}\s. Ensure uniformity in the regex pattern used for hash values.
tests/cases/standalone/information_schema/cluster_info.resultAnalysis chain
Verify the regex pattern update.
The regex pattern for matching hash values was changed to allow 7 to 8 alphanumeric characters. Ensure this change is consistent and aligns with the expected output.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the updated regex pattern is consistent across the file. # Test: Search for the regex pattern updates. Expect: Consistent pattern for hash values. rg --type text 'SQLNESS REPLACE (\s[a-z0-9]{7,8}\s)'Length of output: 83
Script:
#!/bin/bash # Description: Verify the updated regex pattern is consistent across the file. # Test: Search for the regex pattern updates. Expect: Consistent pattern for hash values. rg 'SQLNESS REPLACE (\s[a-z0-9]{7,8}\s)'Length of output: 42
Script:
#!/bin/bash # Description: Verify the presence of any "SQLNESS REPLACE" patterns in the file. # Test: Search for the "SQLNESS REPLACE" pattern. Expect: Identify any similar patterns. rg 'SQLNESS REPLACE'Length of output: 35464
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- tests/cases/standalone/common/function/system.result (1 hunks)
- tests/cases/standalone/common/system/information_schema.result (2 hunks)
- tests/cases/standalone/information_schema/cluster_info.result (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- tests/cases/standalone/common/function/system.result
- tests/cases/standalone/common/system/information_schema.result
Additional comments not posted (1)
tests/cases/standalone/information_schema/cluster_info.result (1)
Line range hint
1-54:
Review the SQL statements and expected outputs.Ensure the SQL statements and expected outputs are correct and complete. The regex pattern changes should align with the expected results.
Verification successful
Ensure the SQL statements and expected outputs are correct and complete.
- The SQL statements and expected outputs in
tests/cases/standalone/information_schema/cluster_info.resultare present and align with the regex replacements.- The regex replacements in the comments should be consistent with the expected results.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the SQL statements and expected outputs are correct and complete. # Test: Search for the SQL statements and expected outputs. rg --type text 'SELECT \* FROM CLUSTER_INFO'Length of output: 75
Script:
#!/bin/bash # Search for the SQL statements and expected outputs. rg 'SELECT \* FROM CLUSTER_INFO'Length of output: 2412
tisonkun
force-pushed
the
fixup-build-info
branch
from
e17de1f
to
383801d
Compare
tisonkun
force-pushed
the
fixup-build-info
branch
from
383801d
to
9f62805
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- tests/cases/distributed/information_schema/cluster_info.result (5 hunks)
- tests/cases/distributed/information_schema/cluster_info.sql (1 hunks)
- tests/cases/standalone/common/function/system.result (1 hunks)
- tests/cases/standalone/common/function/system.sql (1 hunks)
- tests/cases/standalone/common/system/information_schema.result (2 hunks)
- tests/cases/standalone/information_schema/cluster_info.result (3 hunks)
- tests/cases/standalone/information_schema/cluster_info.sql (2 hunks)
Files skipped from review due to trivial changes (3)
- tests/cases/distributed/information_schema/cluster_info.result
- tests/cases/distributed/information_schema/cluster_info.sql
- tests/cases/standalone/common/function/system.sql
Files skipped from review as they are similar to previous changes (3)
- tests/cases/standalone/common/function/system.result
- tests/cases/standalone/common/system/information_schema.result
- tests/cases/standalone/information_schema/cluster_info.result
Additional comments not posted (3)
tests/cases/standalone/information_schema/cluster_info.sql (3)
7-7: Regular expression pattern update looks good.The change to match 7 or 8 characters for hash values is appropriate and aligns with the goal of ensuring accurate build information retrieval.
15-15: Regular expression pattern update looks good.The change to match 7 or 8 characters for hash values is appropriate and aligns with the goal of ensuring accurate build information retrieval.
25-25: Regular expression pattern update looks good.The change to match 7 or 8 characters for hash values is appropriate and aligns with the goal of ensuring accurate build information retrieval.
I hereby agree to the terms of the GreptimeDB CLA.
Refer to a related PR or issue link (optional)
What's changed and what's your intention?
This closes #4331
Checklist
Summary by CodeRabbit
git_dirtyfield togit_cleanininformation_schema.build_infofor clarity.