Add support for modifying field properties prior to model generation #874
Conversation
flaviuvadan
added
semver:patch
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #874 +/- ##
=====================================
Coverage 79.6% 79.6%
=====================================
Files 45 45
Lines 3730 3730
Branches 758 758
=====================================
Hits 2972 2972
Misses 562 562
Partials 196 196 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Flaviu Vadan <flaviuvadan@gmail.com>
Signed-off-by: Flaviu Vadan <flaviuvadan@gmail.com>
There was a problem hiding this comment.
I'm conscious of scripts/spec.py being a bit hacky at this point, but it's to address problems upstream so 🤷♂️ Also, the fix in the underlying generated code is probably a smart union to try to use an int if it looks like an int. But that's for separate investigation/PR
| @@ -1526,7 +1526,7 @@ class HTTPGetAction(BaseModel): | |||
| description=("Custom headers to set in the request. HTTP allows repeated headers."), | |||
| ) | |||
| path: Optional[str] = Field(default=None, description="Path to access on the HTTP server.") | |||
| port: intstr.IntOrString = Field( | |||
| port: int = Field( | |||
| ..., | |||
| description=( | |||
| "Name or number of the port to access on the container. Number must be in" | |||
There was a problem hiding this comment.
Interesting that the description is "name or number" 🤔
There was a problem hiding this comment.
Yea.. and all other ports I looked at, on other objects, are integers. Perhaps it's a backwards compatibility thing that upstream tried to address. I lack the context
Pull Request Checklist
#869 reports how the field
IntOrStringincorrectly renders the port specification into a string that's not accepted by K8s. This PR adds a spec modification strategy that takes the remote Argo OpenAPI JSON specification and modifies the field fromIntOrStringtointeger. This way the correct model is generated, and users know an integer is supposed to be used. Ideally, we'd fix these upstream but this at least unblocks current Hera users.