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.

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

[#3968] improvement(core): Disable KV entity store and optimize CI #3975

Merged
merged 15 commits into from
Jul 8, 2024

Conversation

yuqi1129
Copy link
Contributor

What changes were proposed in this pull request?

  • Disabling support for the KV entity store and adjusting the tests accordingly.
  • Change CI about backend option jdbcBackend

Why are the changes needed?

We are going to deprecate kv entity store

Fix: #3968

Why are the changes needed?

N/A

How was this patch tested?

Existing test.

@yuqi1129 yuqi1129 marked this pull request as draft June 26, 2024 08:54
@yuqi1129 yuqi1129 marked this pull request as ready for review July 1, 2024 14:24
@yuqi1129 yuqi1129 self-assigned this Jul 1, 2024
@yuqi1129 yuqi1129 closed this Jul 3, 2024
@yuqi1129 yuqi1129 reopened this Jul 3, 2024
@yuqi1129 yuqi1129 closed this Jul 3, 2024
@yuqi1129 yuqi1129 reopened this Jul 3, 2024
@@ -61,7 +61,7 @@ jobs:
architecture: [linux/amd64]
java-version: [ 8, 11, 17 ]
test-mode: [ embedded, deploy ]
backend: [ jdbcBackend, kvBackend]
backend: [ mysql, h2]
Copy link
Contributor

Choose a reason for hiding this comment

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

I think maybe we can reduce the pipeline to combine embedded with h2, and deploy with msyql, WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In general, test-mode and backend are different things, I would rather to remove embedded mode in the test-mode to reduce pipelines as backend h2 and MySQL will be run by the user in the real environment, however embedded mode will never exists in real environment.

gradle.properties Outdated Show resolved Hide resolved
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;
import org.mockito.Mockito;

@Disabled
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you disable this?

@@ -41,3 +41,5 @@ pythonVersion = 3.8

# skipDockerTests is used to skip the tests that require Docker to be running.
skipDockerTests = true
# The default backend for the JDBC tests, you set it to h2 or mysql
jdbcBackend = h2
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should figure out a better way for test between h2 and MySQL.

  1. For UT, we can use H2.
  2. For IT, we'd better run both.

So, we don't have to make a configuration there, WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, Add the configuration here is indeed unnecessary as H2 is the default one.

For UT, we can use H2.
For IT, we'd better run both.

Yeah, the strategy is correct. I will check it again.

@jerryshao
Copy link
Contributor

Did you update the docs about the change to use H2 by default?

@yuqi1129
Copy link
Contributor Author

yuqi1129 commented Jul 8, 2024

Did you update the docs about the change to use H2 by default?

The document are not fully changed, I will refine it.

@jerryshao jerryshao merged commit 27ca875 into apache:main Jul 8, 2024
33 checks passed
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.

Deprecate Key-Value entity store and optimize CI
2 participants