Repository navigation
Revert "Codex/fix bling response validation" - #4
Conversation
Mira PR WalkthroughThis PR reverts a previous pull request (#3) that attempted to add response validation logic to the Bling ERP API Python client. It removes custom field types (BlingField, validation helpers) from 50 files reviewed · 2 comments (🛑 2 blockers)
|
![tempont-bot[bot]](https://avatars.githubusercontent.com/u/99068449?s=60&v=4){kind=small}
There was a problem hiding this comment.
Mira Review Summary
This revert removes the custom BlingDatetime type and its AST-rewrite integration in the code generator, replacing it with pydantic's AwareDatetime. While the revert is the explicit intent, it reintroduces two confirmed runtime bugs that were fixed by commits a79782a0 (sentinel normalization), ad4a0f5f (timezone localization), and 8ae63d2a (zero-date injection). AwareDatetime does not normalize Bling's absent-timestamp sentinels (\"0000-00-00 00:00:00\", \"0001-01-01\", empty strings) to None, and it rejects naive datetimes outright instead of localizing them to America/Sao_Paulo. These regressions affect LogisticasObjetosRastreamentoDTO.ultima_alteracao and LogisticasRemessasRastreamentoDTO.ultima_alteracao — the only two generated fields that used BlingDatetime — and will cause ValidationError at runtime when users parse real Bling logistics tracking responses.
Key Issues
| Issue | Location | |
|---|---|---|
| 🔴 | Removal of _normalize_bling_datetime sentinel mapping: zero-date sentinels (0000-00-00, 0001-01-01, etc.) and empty strings from Bling will now cause ValidationError instead of being mapped to None. | src/bling_erp_api/models/generated/schemas/logisticas_objetos.py:100 |
| 🔴 | Removal of _localize_bling_datetime timezone inference: naive timestamps from Bling will be rejected by AwareDatetime instead of being localized to America/Sao_Paulo. | scripts/generate_models.py:915 |
| def _rewrite_date_types(node: ast.ClassDef) -> None: | ||
| """Use Bling date and datetime parsing in generated field annotations.""" | ||
| replacements = {"date": "BlingDate", "AwareDatetime": "BlingDatetime"} | ||
| """Replace ``date`` with ``BlingDate`` in all field annotations.""" |
There was a problem hiding this comment.
Bug
🛑 Blocker — must fix before merge
Removal of AwareDatetime→BlingDatetime rewrite reintroduces Bling datetime quirk
The _rewrite_date_types function previously converted both date → BlingDate and AwareDatetime → BlingDatetime in generated field annotations. The custom BlingDatetime type (defined in fields.py) provided two critical behaviors that pydantic.AwareDatetime lacks:
- Sentinel normalization:
_normalize_bling_datetimemaps Bling's absent-timestamp sentinels ("0000-00-00","0000-00-00 00:00:00","0001-01-01", empty strings) andnulltoNone. Without this, these values will cause aValidationError. - Timezone localization:
_localize_bling_datetimeinterprets naive timestamps inAmerica/Sao_Paulo(including historical DST rules) and preserves explicit offsets.AwareDatetimerejects naive datetimes outright.
Both behaviors were confirmed fixes for real Bling API quirks (commits a79782a0, ad4a0f5f, 8ae63d2a). By removing the AwareDatetime → BlingDatetime rewrite, the generator now outputs AwareDatetime directly in schema models, which will cause ValidationError at runtime when parsing logistics tracking responses that contain zero-date sentinels or naive timestamps.
Prompt for AI Agents
In scripts/generate_models.py, restore the AwareDatetime→BlingDatetime rewrite inside _rewrite_date_types (lines 912-916). Change the docstring to describe both date and AwareDatetime replacements. Revert the loop body: use a replacements dict {"date": "BlingDate", "AwareDatetime": "BlingDatetime"} and check `stmt.id in replacements` then assign `stmt.id = replacements[stmt.id]`. Also restore DATE_IMPORT to `from bling_erp_api.models.fields import BlingDate, BlingDatetime` and restore the original _schema_module_content/_schema_init_content import lines so they use DATE_IMPORT and import BlingDatetime.
Not useful? Reply
@tempont-bot rejectto dismiss this suggestion.
|
|
||
| codigo: str = Field(..., examples=["EC272330554BR"]) | ||
| descricao: str = Field(..., examples=["Criado"]) | ||
| situacao: int = Field(..., examples=["8"]) |
There was a problem hiding this comment.
Bug
🛑 Blocker — must fix before merge
Tracking timestamp field uses AwareDatetime without Bling's sentinel/timezone ha
The field ultima_alteracao in LogisticasObjetosRastreamentoDTO changed from BlingDatetime (custom type with sentinel normalization and timezone localization) to AwareDatetime from pydantic. AwareDatetime does not handle Bling's zero-date sentinels ("0000-00-00 00:00:00", etc.) or naive timestamps ("2020-11-11 16:40:33" without offset). Real BIling API tracking responses that previously parsed successfully will now raise ValidationError. The same regression exists in logisticas_remessas.py at line 140 in LogisticasRemessasRastreamentoDTO.ultima_alteracao. These were the only two generated models that actually used BlingDatetime as a field type.
Prompt for AI Agents
In src/bling_erp_api/models/generated/schemas/logisticas_objetos.py line 100, change `ultima_alteracao: AwareDatetime` to `ultima_alteracao: BlingDatetime`. Update the pydantic import at line 9 to remove `AwareDatetime`, and restore `from bling_erp_api.models.fields import BlingDate, BlingDatetime` at line 7. Apply the identical fix to logisticas_remessas.py line 140 (field type) and line 9 (imports). Also add BlingDatetime back to the _MODEL_NAMESPACE dict in __init__.py.
Not useful? Reply
@tempont-bot rejectto dismiss this suggestion.
1 participant
Reverts #3