[#5289][Improvement] CatalogMysqlDriverIT miss driver url

lsyulong commented 4 weeks ago

What changes were proposed in this pull request?

Why are the changes needed?

[#5289][Improvement] CatalogMysqlDriverIT miss driver url

Fix: #5289

Does this PR introduce any user-facing change?

no

How was this patch tested?

no

lsyulong commented 4 weeks ago

@mchades Hello, I submitted a PR here and it has been successfully built. I see that there are still two integration tests that have not been successful. Do you need me to fix them as well

mchades commented 4 weeks ago

sure, you should resolve these issues to ensure the CI passes.

jerryshao commented 3 weeks ago

@lsyulong https://github.com/apache/gravitino/actions/runs/11675764065/job/32524822188?pr=5431 this failure seems not a flaky CI problem, maybe due to your changes, can you please fix the tests?

lsyulong commented 3 weeks ago

@lsyulong https://github.com/apache/gravitino/actions/runs/11675764065/job/32524822188?pr=5431 this failure seems not a flaky CI problem, maybe due to your changes, can you please fix the tests?https://github.com/apache/gravitino/actions/runs/11675764065/job/32524822188?pr=5431这个失败似乎不是一个片状的 CI 问题，也许是由于你的更改，你能修复测试吗？

Okay, I'll take a look

lsyulong commented 3 weeks ago

@jerryshao I have added some debugging information here, but my local computer cannot debug as there is no corresponding environment. Can you help me run the CI and I will check the logs in the corresponding integration testing

lsyulong commented 3 weeks ago

@jerryshao @mchades Two teachers, I submitted the code and found that the integration test failed to build again. Can you provide me with some help

yuqi1129 commented 3 weeks ago

@lsyulong Please take a look at the PR and the corresponding issue first. https://github.com/apache/gravitino/pull/3294/files There are some small differences between different kinds of JDBC drivers in table/schema meta, please check it first if you still have problems, please ping us.

lsyulong commented 3 weeks ago

@lsyulong Please take a look at the PR and the corresponding issue first. https://github.com/apache/gravitino/pull/3294/files请先看一下 PR 和相应的 Issue。 https://github.com/apache/gravitino/pull/3294/files There are some small differences between different kinds of JDBC drivers in table/schema meta, please check it first if you still have problems, please ping us.不同类型的 JDBC 驱动程序在表/架构元数据中存在一些细微差异，请先检查一下，如果仍有问题，请 ping 我们。

ok, thanks

lsyulong commented 3 weeks ago

@yuqi1129 I saw the RP link you provided, and I replaced the MySQL address in my BaseIT class with 8.0.11 instead of 8.0.23. Is there anything else I couldn't consider? Could you please help me check and correct it

yuqi1129 commented 3 weeks ago

@yuqi1129 I saw the RP link you provided, and I replaced the MySQL address in my BaseIT class with 8.0.11 instead of 8.0.23. Is there anything else I couldn't consider? Could you please help me check and correct it

Let me clarify the original problem.

The version of the JDBC driver is 8.0.23 in BaseIT originally. Someone reported that something is wrong when using 8.0.11, and we have confirmed this issue. This problem was fixed by #3294 (files). However, in one or two versions, someone has removed the related test that uses 8.0.11, so we need to add them back. That's what you are doing.

If we set the version of the JDBC driver to 8.0.11 and there is an exception in the tests, I believe we need to fix those errors. If you need help, you can try and ping us.

lsyulong commented 3 weeks ago

@yuqi1129 I saw the RP link you provided, and I replaced the MySQL address in my BaseIT class with 8.0.11 instead of 8.0.23. Is there anything else I couldn't consider? Could you please help me check and correct it

Let me clarify the original problem.

The version of the JDBC driver is 8.0.23 in BaseIT originally. Someone reported that something is wrong when using 8.0.11, and we have confirmed this issue. This problem was fixed by #3294 (files). However, in one or two versions, someone has removed the related test that uses 8.0.11, so we need to add them back. That's what you are doing.

If we set the version of the JDBC driver to 8.0.11 and there is an exception in the tests, I believe we need to fix those errors. If you need help, you can try and ping us.

@yuqi1129 OK, my understanding is to add back the missing content from the CatalogDoris Driver IT and CatalogMysql Driver IT classes. The rest of the code structure and logic have changed. Do we still need to add this part?

yuqi1129 commented 3 weeks ago

@yuqi1129 I saw the RP link you provided, and I replaced the MySQL address in my BaseIT class with 8.0.11 instead of 8.0.23. Is there anything else I couldn't consider? Could you please help me check and correct it

Let me clarify the original problem. The version of the JDBC driver is 8.0.23 in BaseIT originally. Someone reported that something is wrong when using 8.0.11, and we have confirmed this issue. This problem was fixed by #3294 (files). However, in one or two versions, someone has removed the related test that uses 8.0.11, so we need to add them back. That's what you are doing. If we set the version of the JDBC driver to 8.0.11 and there is an exception in the tests, I believe we need to fix those errors. If you need help, you can try and ping us.

@yuqi1129 OK, my understanding is to add back the missing content from the CatalogDoris Driver IT and CatalogMysql Driver IT classes. The rest of the code structure and logic have changed. Do we still need to add this part?

I don't think so. The only thing you need to do is to set driver version to 8.0.11 in CatalogMysqlDriverIT.java and make all CI PASS.

yuqi1129 commented 3 weeks ago

@lsyulong Can you run the test locally? I suggest you do so locally as CI is very time-consuming and hard to debug.

lsyulong commented 3 weeks ago

@lsyulong Can you run the test locally? I suggest you do so locally as CI is very time-consuming and hard to debug.你可以在本地运行测试吗？我建议您在本地进行此操作，因为 CI 非常耗时且难以调试。

ok

apache / gravitino