[#2678] improvement(test): Add e2e test for jdbc backend #2686
Conversation
core/src/main/java/com/datastrato/gravitino/catalog/CatalogOperationDispatcher.java
Outdated
Show resolved
Hide resolved
| @@ -120,7 +120,7 @@ SchemaPO selectSchemaMetaByCatalogIdAndName( | |||
| + " AND schema_name = #{oldSchemaMeta.schemaName}" | |||
| + " AND metalake_id = #{oldSchemaMeta.metalakeId}" | |||
| + " AND catalog_id = #{oldSchemaMeta.catalogId}" | |||
| + " AND schema_comment = #{oldSchemaMeta.schemaComment}" | |||
| + " AND (schema_comment IS NULL OR schema_comment = #{oldSchemaMeta.schemaComment})" | |||
There was a problem hiding this comment.
This is to fix the problem that schema_comment is nullable
There was a problem hiding this comment.
Should we set this parameter to Not Null and use "" as the default value?
There was a problem hiding this comment.
Comment null and empty are different for many catalogs, so I don't think it's better than the previous design.
|
After this PR, the backend integration test will take as long as 50 minutes. |
|
@xloya |
...ion-test-common/src/test/java/com/datastrato/gravitino/integration/test/util/AbstractIT.java
Outdated
Show resolved
Hide resolved
...ion-test-common/src/test/java/com/datastrato/gravitino/integration/test/util/AbstractIT.java
Outdated
Show resolved
Hide resolved
...ion-test-common/src/test/java/com/datastrato/gravitino/integration/test/util/AbstractIT.java
Outdated
Show resolved
Hide resolved
I'm not sure if it's because of the driver download. During my internal testing, downloading the mysql driver from |
core/src/main/java/com/datastrato/gravitino/storage/relational/service/TableMetaService.java
Outdated
Show resolved
Hide resolved
I'm afraid 50 minutes is as expected. Previous ITs took about 23 minutes on average, as this PR adds another test mode, the total time I think should be double, moreover, the MySQL entity store should be more time-consuming that the KV entity store. Downdowing jars in CI are very fast as they won't be blocked by GFW. |
|
@jerryshao Please help to review it if you have time, thanks |
Could you please help to review it, thanks |
|
Overall LGTM. |
|
I have no further comment, my simple worry is that it will add more CI jobs that make the CI time much longer. |
apache#2686)" This reverts commit 2bfefb9.
apache#2686)" This reverts commit 2bfefb9.
What changes were proposed in this pull request?
Add JDBC entity store in integration test.
Why are the changes needed?
To test whether JDBC backends works.
Fix: #2678
Does this PR introduce any user-facing change?
N/A.
How was this patch tested?
Existing ITs