[bot] Merge master/253cfe83 into rel/dev - #1765
Merged
Merged
Conversation
`CatalogLlmProvider.to_api()` raised `ApiValueError: Invalid value for
`type` (AZURE_FOUNDRY), must be one of ['OPENAI']` for every LLM provider
except one, so `create_llm_provider()` / `update_llm_provider()` were
unusable for OpenAI, AWS Bedrock and Azure Foundry alike (reported against
1.73.0, where the one that worked was Anthropic).
openapi-generator's `python-prior` generator flattens a oneOf's members
into the composed parent model, but for a property that several members
declare it keeps only the *last* member's value. Two things collapse:
* `allowed_values[('type',)]` holds one member's enum, so the parent
accepts exactly one variant - and which one depends on the order of
the oneOf array in the OpenAPI document. That is why the released
1.73.0 accepted only ANTHROPIC while master accepted only OPENAI:
the backend had reordered the array, nothing else changed.
* `openapi_types[prop]` holds one member's class, so the *nested*
`auth` union could not carry Bedrock or Azure Foundry auth either.
Fixed in the custom templates so it survives regeneration, rather than
in the generated sources or the spec. A spec-side `discriminator` was
ruled out first: v6.6.0 emits the identical collapsed model with and
without one (the existing afm discriminators generate an empty
`discriminator_value_class_map`). Generator v7's `python` generator does
emit a proper `Union`, so this is interim until that migration.
`set_attribute` now validates against the union of the members' types and
enums. This loses no checking - the value is still validated against the
composed schemas by `validate_get_composed_info`; only the parent's bogus
narrowed constraints are widened. `attempt_convert_item` also had to stop
re-raising on the first candidate class when `must_convert` is set, since
a union property has several candidates and the first is not necessarily
the match.
The defect was repo-wide: an audit found 25 collapsed enums and 47
collapsed types across 161 composed models. Fixing it generically retires
the per-site workarounds it has been accumulating, starting with the
`NotificationChannelDestination.allowed_values` monkeypatch removed in the
following commit. Only `model_utils.py` changes in the generated tree.
`CatalogSmtp` and `CatalogDefaultSmtp` had been sitting commented out with a
TODO blaming the generated client:
# TODO: there is an issue with generated client which causes these two
# classes to fail. type in declarative_notification_channel_destination.py
# contains only WEBHOOK as valid value
That was the oneOf-flattening defect fixed in the previous commit, so
`destination` could only ever be a webhook. With the generated composed model
now accepting every member, add the missing destinations and widen the union:
* CatalogSmtp - custom SMTP server
* CatalogDefaultSmtp - the platform's own mail server
* CatalogInPlatform - in-platform notifications
`destination` becomes `CatalogNotificationChannelDestination`, the union the
commented-out code intended.
Also drop the runtime monkeypatch that re-populated
`NotificationChannelDestination.allowed_values[("type",)]`. It treated one
symptom of the same defect from outside the generated client; the template fix
covers every collapsed composed model, so patching class attributes at import
time is no longer needed.
Reading a channel needs an explicit `from_api` rather than cattrs: the four
destination classes have no uniquely-required field to disambiguate a union on
(IN_PLATFORM carries nothing but its type), so dispatch on `type` the way
`_provider_config_from_api` does for LLM providers. It keeps only the fields
each class declares, because the API sends some we do not model - reading a
webhook returns `has_secret_key` - and dropping those matches what cattrs did
before and keeps reads working when the API grows a field.
fix: accept every oneOf variant in generated composed models
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## rel/dev #1765 +/- ##
===========================================
+ Coverage 80.65% 80.83% +0.17%
===========================================
Files 272 272
Lines 19369 19416 +47
===========================================
+ Hits 15622 15694 +72
+ Misses 3747 3722 -25 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
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.
🚀 Automated PR to perform merge from master into rel/dev with changes up to 253cfe8 (created by https://github.com/gooddata/gooddata-python-sdk/actions/runs/33081399187).