[Tech Debt]Add cross-language resource consistency checking tool#579
[Tech Debt]Add cross-language resource consistency checking tool#579da-daken wants to merge 6 commits intoapache:mainfrom
Conversation
github-actions
bot
added
doc-not-needed
|
hi @xintongsong , This push did not trigger CI. I noticed that the error message in CI indicates that astral-sh/setup-uv@v4 is not allowed, but I didn't add it. How should I resolve this issue? and hope you can review when you have time! |
xintongsong
force-pushed
the
resource_ci_check
branch
from
9e29579 to
af72ae0
Compare
|
@da-daken Sorry for the late response. It took us a bit time to fix the CI issue. Now the CI should be properly triggered. |
|
@xintongsong Thank you for resolving the CI issue. Now the CI has all passed. You can review it when you have time. The plan is for both the Java side and the Python side to check whether their own classes exist, and then a consistency script will determine if both sides are consistent. This way, we can verify that the classes on both sides can be correctly loaded. |
.github/workflows/ci.yml
Outdated
| echo "$HOME/.local/bin" >> $GITHUB_PATH | ||
| - name: Check code style | ||
| run: ./tools/lint.sh -c | ||
| - name: Check ResourceName consistency (Java vs Python) |
There was a problem hiding this comment.
I think that placing consistency checks for resource names within code style checks is not entirely appropriate, because code style typically refers to the checking of coding conventions and rules within a single language. I think it is more appropriate to include it in the cross-language test.
python/pyproject.toml
Outdated
| test = [ | ||
| "pytest==8.4.0", | ||
| "apache-flink>=1.20", | ||
| "cloudpickle", |
There was a problem hiding this comment.
If the consistency check for resource names is moved to the cross-language test, these additional dependencies are unnecessary because we manually install apache-flink in tools/e2e.sh.
There was a problem hiding this comment.
yes, you're right! thank you for your review! I will commit soon
.github/workflows/ci.yml
Outdated
| tools/e2e.sh | ||
| - name: Check ResourceName consistency (Java vs Python) | ||
| working-directory: python | ||
| run: uv run --no-sync python ../tools/check_resource_consistency.py No newline at end of file |
There was a problem hiding this comment.
I think we can add this test to e2e.sh instead of creating a separate job stage.
There was a problem hiding this comment.
Yes, it looks cleaner this way, I will move it to e2e.sh
|
LGTM. Please take a look at your convenience @xintongsong . |
3 participants
Linked issue: #xxx
Purpose of change
to fix that 'It would be nice check in CI that the referenced class actually exist, and names in Java and Python are consistent.'
Tests
API
no API
doc-neededdoc-not-neededdoc-included