feat: pydantic models for type checking S3 uploads #253
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds a Pydantic models package and integrates pre-upload JSON schema validation into the S3 client; also updates packaging to include the models and bumps runtime dependency on Pydantic. Changes
Sequence DiagramsequenceDiagram
participant User
participant S3Client
participant FileSystem as File System
participant Validator as Pydantic Validator
participant S3 as AWS S3
User->>S3Client: upload_json_file(local_path, s3_name)
S3Client->>S3Client: _get_validator(s3_name)
S3Client-->>S3Client: returns validator model or None
alt validator found
S3Client->>FileSystem: read JSON from local_path
FileSystem-->>S3Client: json_data
S3Client->>Validator: model_validate(json_data)
alt validation passes
Validator-->>S3Client: validated object
S3Client->>S3: upload file
S3-->>S3Client: upload success
else validation fails
Validator-->>S3Client: ValidationError
S3Client-->>User: log error & abort upload
end
else no validator
S3Client->>S3: upload file (no validation)
S3-->>S3Client: upload success
end
S3Client-->>User: return result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@apps/dataflow/src/clients/_s3_client.py`:
- Around line 50-55: The _get_validator function currently uses suffix-matching
(s3_object_name.endswith(pattern)) which can select the wrong validator for
names like "old_buildings.json"; change it to compare the filename basename
exactly against the pattern instead. Update _get_validator to compute the
basename (e.g., via os.path.basename or Path(...).name) and use equality
(basename == pattern) when iterating SCHEMA_VALIDATORS, keeping the same return
of the validator or None; reference the _get_validator function and
SCHEMA_VALIDATORS when making this change.
In `@apps/dataflow/src/models/buildings.py`:
- Line 27: The field declaration entrances: list | None = None is missing a
generic type parameter; update the type to include the element type (for example
entrances: list[str] | None = None or entrances: list[Entrance] | None = None)
to match the actual contents used in the module and satisfy the type
checker—modify the entrances attribute in models/buildings.py accordingly,
replacing plain list with the appropriate list[T] generic.
- Around line 9-10: The import of GeoCoordinate is currently guarded by
TYPE_CHECKING which removes it at runtime and breaks Pydantic nested-model
validation for fields like label_position, shapes, and hitbox; move the
GeoCoordinate import out of the TYPE_CHECKING block so the class is available at
runtime (i.e., import GeoCoordinate normally from models._common) so Pydantic
can validate those nested fields in the Building model.
In `@apps/dataflow/src/models/floorplans.py`:
- Around line 9-10: The imports for Floor and GeoCoordinate are only under the
TYPE_CHECKING guard which prevents Pydantic from seeing the real classes at
runtime and breaks nested model validation for fields like label_position,
floor, and coordinates; move the from models._common import Floor, GeoCoordinate
import statements out of the TYPE_CHECKING block so the actual classes are
available at runtime (or alternatively switch those field annotations to
forward-reference strings and call update_forward_refs where appropriate),
ensuring symbols Floor and GeoCoordinate are resolvable by Pydantic during
validation.
In `@apps/dataflow/src/models/graph.py`:
- Around line 9-10: The TYPE_CHECKING guard prevents Pydantic from seeing nested
model classes at runtime; move the imports of Floor, GeoCoordinate, and
LocalPosition out of the TYPE_CHECKING block so they are imported at runtime
(i.e., import the actual classes at module top-level rather than only under
TYPE_CHECKING) so Pydantic validation for any usages in this module (e.g., Graph
models or fields that reference Floor, GeoCoordinate, LocalPosition) works
correctly.
In `@apps/dataflow/src/models/placements.py`:
- Around line 9-10: The TYPE_CHECKING-only imports for GeoCoordinate and
LocalPosition prevent Pydantic from seeing those nested model classes at
runtime; move the imports for GeoCoordinate and LocalPosition out of the
TYPE_CHECKING block so they are actual runtime imports (i.e., import from
models._common at module level), ensuring any Pydantic model fields in this
module that reference GeoCoordinate and LocalPosition resolve correctly during
validation.
🧹 Nitpick comments (1)
apps/dataflow/src/models/graph.py (1)
19-19: Consider defining a typed model forto_floor_infoif structure is known.Using
dict[str, Any]bypasses Pydantic's strict validation. If the structure of floor transition info is known and consistent, consider creating a dedicated model for it to maintain validation consistency with the rest of the schema.
|
Note Docstrings generation - SUCCESS |
coderabbitai bot
added a commit
that referenced
this pull request
Jan 25, 2026
Docstrings generation was requested by @Richard-31415. * #253 (comment) The following files were modified: * `apps/dataflow/src/clients/_s3_client.py`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.