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

[#478] feat(core, catalogs): Refactor catalog configurations #489

Merged
merged 25 commits into from
Oct 12, 2023
Merged

[#478] feat(core, catalogs): Refactor catalog configurations #489

merged 25 commits into from
Oct 12, 2023

Conversation

yuqi1129
Copy link
Contributor

@yuqi1129 yuqi1129 commented Oct 10, 2023 •
edited by jerryshao
Loading

What changes were proposed in this pull request?

In this PR, we introduce the following points:

  • Remove the catalog-related configuration from Graviton server configuration file.
  • Support setting catalog configuration with a separate config file located in catalogs module
  • Provide catalog-related configurations for a specific engine with an unique prefix.

Why are the changes needed?

Fix: #478
Fix: #196

Does this PR introduce any user-facing change?

N/A

How was this patch tested?

New UTs

@yuqi1129 yuqi1129 marked this pull request as draft October 10, 2023 14:21
@yuqi1129 yuqi1129 self-assigned this Oct 10, 2023
@github-actions
Copy link

github-actions bot commented Oct 10, 2023 •
edited
Loading

Code Coverage Report

Overall Project 62.26% -0.7% 🟢
Files changed 41.57% 🔴

Module Coverage
core 74.56% -2.63% 🔴
catalog-hive 64.57% 🟢
Files
Module File Coverage
core BaseCatalog.java 75.9% 🟢
CatalogManager.java 61.73% -15.37% 🔴
AuxiliaryServiceManager.java 59.17% -0.73% 🟢
IsolatedClassLoader.java 41.41% -26.04% 🔴
catalog-hive HiveCatalogOperations.java 67.98% 🟢

@yuqi1129 yuqi1129 marked this pull request as ready for review October 11, 2023 08:28
@jerryshao
Copy link
Contributor

Is it ready for review?

@yuqi1129
Copy link
Contributor Author

Is it ready for review?

Yes

@yuqi1129
Copy link
Contributor Author

@jerryshao, please take a look. thanks

@yuqi1129 yuqi1129 requested a review from mchades October 12, 2023 02:43
@yuqi1129
Copy link
Contributor Author

@jerryshao
All have been addressed, please help to review again. Thanks

@jerryshao jerryshao merged commit 4d9e7a9 into apache:main Oct 12, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FANNG1 FANNG1 FANNG1 left review comments

@jerryshao jerryshao jerryshao approved these changes

@mchades mchades Awaiting requested review from mchades

Assignees

@yuqi1129 yuqi1129

Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@yuqi1129 @jerryshao @FANNG1