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.

Sign up for GitHub

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

Jump to bottom

[Improvement] Property name consistency check in the graviton.conf.template file #390

Closed
xunliu opened this issue Sep 13, 2023 · 0 comments · Fixed by #408
Closed

[Improvement] Property name consistency check in the graviton.conf.template file #390

xunliu opened this issue Sep 13, 2023 · 0 comments · Fixed by #408
Assignees
@xunliu
Labels
improvement Improvements on everything

Comments

@xunliu
Copy link
Member

xunliu commented Sep 13, 2023

What would you like to be improved?

Because property names in the graviton.conf.template file need to be kept consistent with com/datastrato/graviton/Configs.java and com/datastrato/graviton/server/ServerConfig.java, Otherwise Graviton will not be able to read graviton.conf file correctly.

How should we improve?

We need to add test cases to keep them consistent.

I think we can use all property name variants of the config.java and ServerConfig.java to check in the graviton.conf.
@xunliu xunliu added the improvement Improvements on everything label Sep 13, 2023
@xunliu xunliu self-assigned this Sep 13, 2023
@xunliu xunliu mentioned this issue Sep 18, 2023
Merged
jerryshao pushed a commit that referenced this issue Sep 18, 2023
@xunliu
[#390] improve(test): Consistency check in the graviton.conf file (#408)
0d00735 
### What changes were proposed in this pull request?

+ Add ServerConfigTest to keep consistency check in the graviton.conf
file.
+ Use the JAVA reflect method to get all property names from
`ServerConfig.java` and `Configs.java` files.

### Why are the changes needed?

Because property names in the `graviton.conf.template` file need to be
kept consistent with `com/datastrato/graviton/Configs.java` and
`com/datastrato/graviton/server/ServerConfig.java`,
Otherwise, Graviton will not be able to read `graviton.conf` file
correctly.

Fix: #390 

### Does this PR introduce _any_ user-facing change?

N/A

### How was this patch tested?

Added `ServerConfigTest` in the test cases.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@xunliu xunliu

Labels
improvement Improvements on everything
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[#390] improve(test): Consistency check in the graviton.conf file
1 participant
@xunliu