Skip to content

[#3302][Sub-Task] StarRocks catalog E2E test #7792

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
Merged
merged 2 commits into from
Jul 31, 2025
Merged

[#3302][Sub-Task] StarRocks catalog E2E test #7792

merged 2 commits into from
Jul 31, 2025

Conversation

liunaijie
Copy link
Member

What changes were proposed in this pull request?

add StarRocks Catalog Implement

Why are the changes needed?

To support StarRocks Catalog.

Does this PR introduce any user-facing change?

No

How was this patch tested?

By E2E test

This was referenced Jul 24, 2025
Merged
Merged
@liunaijie liunaijie force-pushed the feature/catalog_sr3 branch from b7e5135 to d9316a0 Compare July 28, 2025 05:25
yuqi1129 pushed a commit that referenced this pull request Jul 28, 2025
@liunaijie
[#3302][Sub-Task] StarRocks catalog Table/DB ops (#7738)
71936c8 
### What changes were proposed in this pull request?

add StarRocks Catalog Implement

### Why are the changes needed?

To support StarRocks Catalog.

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

No

### How was this patch tested?

By E2E test, the test is in another pr

#7792
@liunaijie liunaijie force-pushed the feature/catalog_sr3 branch from e8b7c2c to b54bfcc Compare July 30, 2025 07:07
yuqi1129 pushed a commit that referenced this pull request Jul 30, 2025
@liunaijie
[#3302][Sub-Task] StarRocks catalog Partition ops (#7791)
8b4d86a 
<!--
1. Title: [#<issue>] <type>(<scope>): <subject>
   Examples:
     - "[#123] feat(operator): support xxx"
     - "[#233] fix: check null before access result in xxx"
     - "[MINOR] refactor: fix typo in variable name"
     - "[MINOR] docs: fix typo in README"
     - "[#255] test: fix flaky test NameOfTheTest"
   Reference: https://www.conventionalcommits.org/en/v1.0.0/
2. If the PR is unfinished, please mark this PR as draft.
-->

### What changes were proposed in this pull request?

add StarRocks Catalog Implement

### Why are the changes needed?

To support StarRocks Catalog.

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

No

### How was this patch tested?

By E2E test, the test is in another pr

#7792
@yuqi1129
Copy link
Contributor

@liunaijie
Please resolve the conflict if you have free time. Thanks.

@liunaijie liunaijie force-pushed the feature/catalog_sr3 branch from b54bfcc to 9d73f43 Compare July 30, 2025 09:41
liunaijie
liunaijie commented Jul 30, 2025
View reviewed changes
...s/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/config/JdbcConfig.java Outdated
@@ -65,7 +66,7 @@ public class JdbcConfig extends Config {
.doc("The password of the Jdbc connection")
.version(ConfigConstants.VERSION_0_3_0)
.stringConf()
.checkValue(StringUtils::isNotBlank, ConfigConstants.NOT_BLANK_ERROR_MSG)
.checkValue(Objects::nonNull, ConfigConstants.NOT_BLANK_ERROR_MSG)
Copy link
Member Author

@liunaijie liunaijie Jul 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The alert message will update to NOT_NULL_ERROR_MSG

yuqi1129
yuqi1129 reviewed Jul 30, 2025
View reviewed changes
...common/src/test/java/org/apache/gravitino/integration/test/container/StarRocksContainer.java
public static final String DEFAULT_IMAGE = "starrocks/allin1-ubuntu:3.3-latest";
public static final String HOST_NAME = "gravitino-ci-starrocks";
public static final String USER_NAME = "root";
public static final String PASSWORD = "";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use a meaningful password instead of an empty one? is that any other consideration?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@liunaijie
Please leave a comment here.

Copy link
Member Author

@liunaijie liunaijie Jul 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's default password.
image

...common/src/test/java/org/apache/gravitino/integration/test/container/StarRocksContainer.java Outdated
public static final int FE_HTTP_PORT = 8030;
public static final int FE_MYSQL_PORT = 9030;

private static final String STARROCKS_FE_PATH = "/opt/apache-starrocks/fe/log";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Starrocks is not an Apache project, you can change it to other name.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

...s/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/config/JdbcConfig.java Outdated
@@ -65,7 +66,7 @@ public class JdbcConfig extends Config {
.doc("The password of the Jdbc connection")
.version(ConfigConstants.VERSION_0_3_0)
.stringConf()
.checkValue(StringUtils::isNotBlank, ConfigConstants.NOT_BLANK_ERROR_MSG)
.checkValue(Objects::nonNull, ConfigConstants.NOT_BLANK_ERROR_MSG)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am very confused about why we need to make this change.

core/src/main/java/org/apache/gravitino/Config.java
@@ -198,7 +198,7 @@ public void loadFromMap(Map<String, String> map, Predicate<String> predicate) {
(k, v) -> {
String trimmedK = k.trim();
String trimmedV = v.trim();
if (!trimmedK.isEmpty() && !trimmedV.isEmpty()) {
if (!trimmedK.isEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When set property, empty key is meanless, we can filter it, but maybe the empty value is valid is sometime. I think we don't need filter it in the framework.

For example I want take a spike for Gravitino. I download it and want to connect my dev mysql, the password is empty string.
Then I can't connect it as I just use an empty password.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

@liunaijie liunaijie force-pushed the feature/catalog_sr3 branch from 9d73f43 to 10488ee Compare July 30, 2025 10:00
jerqi pushed a commit to qqqttt123/gravitino that referenced this pull request Jul 30, 2025
@liunaijie @jerqi
[apache#3302][Sub-Task] StarRocks catalog Table/DB ops (apache#7738)
da0ebe1 
### What changes were proposed in this pull request?

add StarRocks Catalog Implement

### Why are the changes needed?

To support StarRocks Catalog.

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

No

### How was this patch tested?

By E2E test, the test is in another pr

apache#7792
jerqi pushed a commit to qqqttt123/gravitino that referenced this pull request Jul 30, 2025
@liunaijie @jerqi
[apache#3302][Sub-Task] StarRocks catalog Partition ops (apache#7791)
ac31c14 
<!--
1. Title: [#<issue>] <type>(<scope>): <subject>
   Examples:
     - "[apache#123] feat(operator): support xxx"
     - "[apache#233] fix: check null before access result in xxx"
     - "[MINOR] refactor: fix typo in variable name"
     - "[MINOR] docs: fix typo in README"
     - "[apache#255] test: fix flaky test NameOfTheTest"
   Reference: https://www.conventionalcommits.org/en/v1.0.0/
2. If the PR is unfinished, please mark this PR as draft.
-->

### What changes were proposed in this pull request?

add StarRocks Catalog Implement

### Why are the changes needed?

To support StarRocks Catalog.

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

No

### How was this patch tested?

By E2E test, the test is in another pr

apache#7792
@yuqi1129 yuqi1129 merged commit aefb274 into apache:main Jul 31, 2025
30 checks passed
tsungchih pushed a commit to tsungchih/gravitino that referenced this pull request Aug 2, 2025
@liunaijie @tsungchih
[apache#3302][Sub-Task] StarRocks catalog Table/DB ops (apache#7738)
efd1307 
### What changes were proposed in this pull request?

add StarRocks Catalog Implement

### Why are the changes needed?

To support StarRocks Catalog.

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

No

### How was this patch tested?

By E2E test, the test is in another pr

apache#7792
tsungchih pushed a commit to tsungchih/gravitino that referenced this pull request Aug 2, 2025
@liunaijie @tsungchih
[apache#3302][Sub-Task] StarRocks catalog Partition ops (apache#7791)
730e814 
<!--
1. Title: [#<issue>] <type>(<scope>): <subject>
   Examples:
     - "[apache#123] feat(operator): support xxx"
     - "[apache#233] fix: check null before access result in xxx"
     - "[MINOR] refactor: fix typo in variable name"
     - "[MINOR] docs: fix typo in README"
     - "[apache#255] test: fix flaky test NameOfTheTest"
   Reference: https://www.conventionalcommits.org/en/v1.0.0/
2. If the PR is unfinished, please mark this PR as draft.
-->

### What changes were proposed in this pull request?

add StarRocks Catalog Implement

### Why are the changes needed?

To support StarRocks Catalog.

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

No

### How was this patch tested?

By E2E test, the test is in another pr

apache#7792
tsungchih pushed a commit to tsungchih/gravitino that referenced this pull request Aug 2, 2025
@liunaijie @tsungchih
[apache#3302][Sub-Task] StarRocks catalog E2E test (apache#7792)
fe3d2f7 
### What changes were proposed in this pull request?

add StarRocks Catalog Implement

### Why are the changes needed?

To support StarRocks Catalog.

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

No

### How was this patch tested?

By E2E test
yuqi1129 pushed a commit to yuqi1129/gravitino that referenced this pull request Aug 15, 2025
@liunaijie @yuqi1129
[apache#3302][Sub-Task] StarRocks catalog Table/DB ops (apache#7738)
3ecd659 
### What changes were proposed in this pull request?

add StarRocks Catalog Implement

### Why are the changes needed?

To support StarRocks Catalog.

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

No

### How was this patch tested?

By E2E test, the test is in another pr

apache#7792
yuqi1129 pushed a commit to yuqi1129/gravitino that referenced this pull request Aug 15, 2025
@liunaijie @yuqi1129
[apache#3302][Sub-Task] StarRocks catalog Partition ops (apache#7791)
599f30c 
<!--
1. Title: [#<issue>] <type>(<scope>): <subject>
   Examples:
     - "[apache#123] feat(operator): support xxx"
     - "[apache#233] fix: check null before access result in xxx"
     - "[MINOR] refactor: fix typo in variable name"
     - "[MINOR] docs: fix typo in README"
     - "[apache#255] test: fix flaky test NameOfTheTest"
   Reference: https://www.conventionalcommits.org/en/v1.0.0/
2. If the PR is unfinished, please mark this PR as draft.
-->

### What changes were proposed in this pull request?

add StarRocks Catalog Implement

### Why are the changes needed?

To support StarRocks Catalog.

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

No

### How was this patch tested?

By E2E test, the test is in another pr

apache#7792
yuqi1129 pushed a commit to yuqi1129/gravitino that referenced this pull request Aug 15, 2025
@liunaijie @yuqi1129
[apache#3302][Sub-Task] StarRocks catalog E2E test (apache#7792)
70da854 
### What changes were proposed in this pull request?

add StarRocks Catalog Implement

### Why are the changes needed?

To support StarRocks Catalog.

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

No

### How was this patch tested?

By E2E test
yuqi1129 pushed a commit to yuqi1129/gravitino that referenced this pull request Aug 18, 2025
@liunaijie @yuqi1129
[apache#3302][Sub-Task] StarRocks catalog Table/DB ops (apache#7738)
7102d45 
### What changes were proposed in this pull request?

add StarRocks Catalog Implement

### Why are the changes needed?

To support StarRocks Catalog.

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

No

### How was this patch tested?

By E2E test, the test is in another pr

apache#7792
yuqi1129 pushed a commit to yuqi1129/gravitino that referenced this pull request Aug 18, 2025
@liunaijie @yuqi1129
[apache#3302][Sub-Task] StarRocks catalog Partition ops (apache#7791)
fedc289 
<!--
1. Title: [#<issue>] <type>(<scope>): <subject>
   Examples:
     - "[apache#123] feat(operator): support xxx"
     - "[apache#233] fix: check null before access result in xxx"
     - "[MINOR] refactor: fix typo in variable name"
     - "[MINOR] docs: fix typo in README"
     - "[apache#255] test: fix flaky test NameOfTheTest"
   Reference: https://www.conventionalcommits.org/en/v1.0.0/
2. If the PR is unfinished, please mark this PR as draft.
-->

### What changes were proposed in this pull request?

add StarRocks Catalog Implement

### Why are the changes needed?

To support StarRocks Catalog.

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

No

### How was this patch tested?

By E2E test, the test is in another pr

apache#7792
yuqi1129 pushed a commit to yuqi1129/gravitino that referenced this pull request Aug 18, 2025
@liunaijie @yuqi1129
[apache#3302][Sub-Task] StarRocks catalog E2E test (apache#7792)
4687d81 
### What changes were proposed in this pull request?

add StarRocks Catalog Implement

### Why are the changes needed?

To support StarRocks Catalog.

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

No

### How was this patch tested?

By E2E test
jerryshao pushed a commit that referenced this pull request Aug 18, 2025
@liunaijie @jerryshao
[#3302][Sub-Task] StarRocks catalog Table/DB ops (#7738)
789504b 
### What changes were proposed in this pull request?

add StarRocks Catalog Implement

### Why are the changes needed?

To support StarRocks Catalog.

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

No

### How was this patch tested?

By E2E test, the test is in another pr

#7792
jerryshao pushed a commit that referenced this pull request Aug 18, 2025
@liunaijie @jerryshao
[#3302][Sub-Task] StarRocks catalog Partition ops (#7791)
8cdfda2 
<!--
1. Title: [#<issue>] <type>(<scope>): <subject>
   Examples:
     - "[#123] feat(operator): support xxx"
     - "[#233] fix: check null before access result in xxx"
     - "[MINOR] refactor: fix typo in variable name"
     - "[MINOR] docs: fix typo in README"
     - "[#255] test: fix flaky test NameOfTheTest"
   Reference: https://www.conventionalcommits.org/en/v1.0.0/
2. If the PR is unfinished, please mark this PR as draft.
-->

### What changes were proposed in this pull request?

add StarRocks Catalog Implement

### Why are the changes needed?

To support StarRocks Catalog.

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

No

### How was this patch tested?

By E2E test, the test is in another pr

#7792
jerryshao pushed a commit that referenced this pull request Aug 18, 2025
@liunaijie @jerryshao
[#3302][Sub-Task] StarRocks catalog E2E test (#7792)
ca5b817 
### What changes were proposed in this pull request?

add StarRocks Catalog Implement

### Why are the changes needed?

To support StarRocks Catalog.

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

No

### How was this patch tested?

By E2E test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yuqi1129 yuqi1129 yuqi1129 approved these changes

Assignees

@liunaijie liunaijie

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@liunaijie @yuqi1129