airbyte-ci: fix format cli usage in airbyte-enterprise #43386
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
postamar
force-pushed
the
postamar/airbyte-ci-support-format-cli-usage-in-airbyte-enterprise
branch
from
f05a477
to
7c508ed
Compare
postamar
force-pushed
the
postamar/airbyte-ci-support-format-cli-usage-in-airbyte-enterprise
branch
from
7c508ed
to
84e6a98
Compare
| @@ -62,7 +67,7 @@ class Formatter(Enum): | |||
| WARM_UP_INCLUSIONS = { | |||
| Formatter.JAVA: [ | |||
| "spotless-maven-pom.xml", | |||
| "tools/gradle/codestyle/java-google-style.xml", | |||
| "tools/", # Whole directory instead of just 'java-google-style.xml' to support airbyte-enterprise. | |||
There was a problem hiding this comment.
I'm not sure why this change is required. And I think it could impact performance as we mount more things during the "warm up" phase.
There was a problem hiding this comment.
In airbyte-enterprise we have a symlink to tools and including a file inside of that only works if the symlink works.
I understand why you'd be worried but tools only contains 1.2 MB of files which basically never change. I think it's OK.
| include = WARM_UP_INCLUSIONS.get(formatter) | ||
| if not include: | ||
| return None | ||
| include = include + [f"{AIRBYTE_SUBMODULE_DIR_NAME}/{i}" for i in include] |
There was a problem hiding this comment.
Shouldn't we check thaht AIRBYTE_SUBMODULE_DIR_NAME exists first? I'm not sure if dagger fails if you tell it to include not existing path (as the submodule does not exists in the main repo)
There was a problem hiding this comment.
No, conveniently enough, it's fine if the path doesn't exist!
postamar
deleted the
postamar/airbyte-ci-support-format-cli-usage-in-airbyte-enterprise
branch
What
Fixes bugs which prevented
airbyte-ci format fix allfrom working in airbyte-enterprise when run straight from the CLI.Also includes a QoL fix which makes maven run in batch mode instead of interactive mode, this makes the execution logs nicer.
How
Inclusion and exclusion rules have been corrected to handle symlinks propertly.
Review guide
One commit at a time.
User Impact
None.
Can this PR be safely reverted and rolled back?