chore(python): uncomment ruff version check in python-v2 Dockerfiles#13880
chore(python): uncomment ruff version check in python-v2 Dockerfiles#13880Swimburger merged 2 commits intomainfrom
Conversation
Co-Authored-By: Niels Swimberghe <3382717+Swimburger@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
There was a problem hiding this comment.
Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit in Settings → Usage.
Once credits are available, reopen this pull request to trigger a review.
|
Is there a reason we shouldn't do this? |
|
No reason not to — it's a pure build-time sanity check that catches broken ruff installations before the image ships. The only caveat is that this PR should be merged after #13871 (which fixes the ruff PATH in pydantic-model), otherwise the pydantic-model Docker build will fail because ruff won't be found at the wrong PATH. The sdk Dockerfile already has the correct PATH so it's fine either way. |
Co-Authored-By: Niels Swimberghe <3382717+Swimburger@users.noreply.github.com>
There was a problem hiding this comment.
Devin Review found 1 new potential issue.
🐛 1 issue in files not directly in the diff
🐛 types/mod.rs declares modules for inlined types that have no generated file (generators/rust/model/src/generateModels.ts:47-51)
The new computeInlinedUnionVariantTypeIds logic in generateModels.ts:47-51 skips generating .rs files for types in context.inlinedUnionVariantTypeIds. However, ModelGeneratorCli.ts:132-153 (generateTypesModFile) iterates over ALL context.ir.types without checking inlinedUnionVariantTypeIds, so it emits pub mod <inlined_type>; and pub use <inlined_type>::<TypeName>; for types that have no corresponding file. This causes a Rust compilation error (file not found for module). The test snapshot confirms this: types_event_payload.rs is deleted, yet the mod.rs generation code would still declare it.
Same bug also affects SdkGeneratorCli.ts:614
The SDK's buildTypesModule has the identical problem — it iterates all context.ir.types entries without filtering inlined types. Since inlinedUnionVariantTypeIds lives on the temporary ModelGeneratorContext created via toModelGeneratorContext(), the SDK context doesn't even have access to the set.
View 5 additional findings in Devin Review.
Description
Uncomments the
RUN ruff --versionbuild-time sanity check in the python-v2 generator Dockerfiles. This check was previously commented out, meaning a broken ruff installation would go undetected until runtime. Enabling it causes the Docker build to fail fast if ruff is not correctly installed or not onPATH.Changes Made
RUN ruff --versioningenerators/python-v2/sdk/DockerfileRUN ruff --versioningenerators/python-v2/pydantic-model/DockerfileHuman Review Checklist
pydantic-model/Dockerfilewill fail to build as-is. TheENV PATHon line 4 is set to/root/.cargo/bin(incorrect), but ruff installs to/root/.local/bin. Uncommenting the version check will cause the build to error becauseruffwon't be found. This PR should either be merged after the PATH fix PR (fix(python): correct ruff PATH in python-v2/pydantic-model Dockerfile #13871), or the PATH fix should be included here.sdk/DockerfilePATH (/root/.local/bin) is correct — this one should work.Testing
PATHconfiguration in both DockerfilesLink to Devin session: https://app.devin.ai/sessions/371cc6bdbad447bb9161b29b0ab8ec28
Requested by: @Swimburger