Update container #98
Update container #98
Conversation
mdellweg
force-pushed
the
update_container
branch
5 times, most recently
from
09f7286 to
a032ed7
Compare
mdellweg
force-pushed
the
update_container
branch
3 times, most recently
from
04312c5 to
b9099c8
Compare
gen-client.sh
Outdated
| FIX_TASK_CREATED_RESOURCES_FILTER='(.components.schemas.TaskResponse|select(.)|.properties.created_resources.items) |= {"$oneOf":[{type:"null"},.]}' | ||
| FIX_TASK_ERROR_FILTER='(.components.schemas.TaskResponse|select(.)|.properties.error) |= (del(.readOnly) | .additionalProperties.type = "string")' |
There was a problem hiding this comment.
Can you explain what these filters are doing?
There was a problem hiding this comment.
Both of them are necessary, because the newer templates do more sophisticated validation on the http responses than before.
The first one is to allow the created_resources list to contain null entries, which happens when we delete a resource formerly locked by the task. (It is not even evident to me whether it's better to not report them on the server side anymore, but as a matter of fact, current pulp violates the schema here.)
The second one, I'm not even sure this is the right thing to do. But what is does: It removes the readOnly property from the error attribute and changes its additionalProperties to be strings. I encountered this in some tests, where the bindings were unable to instantiate a TaskResponse object for the readOnly thing and it's unclear to me why this should be an issue.
There was a problem hiding this comment.
I wrote these filters as Variables (Constants) because they may be useful to the other generators too. Who knows...
| @@ -433,12 +464,15 @@ conf = {{{packageName}}}.Configuration( | |||
| auth = {} | |||
| {{#authMethods}} | |||
| {{#isApiKey}} | |||
| if '{{keyParamName}}' in self.api_key: | |||
| if '{{name}}' in self.api_key{{#vendorExtensions.x-auth-id-alias}} or '{{.}}' in self.api_key{{/vendorExtensions.x-auth-id-alias}}: | |||
There was a problem hiding this comment.
Is this what we are modifying from the original template?
There was a problem hiding this comment.
It's been too long...
I kind of did a manual 3-way merge pulling the new and old versions of the templates from git to see what we changed originally.
This honestly looks like an improvement of the auth code on the o-a-generator side.
| _content_type: Optional[StrictStr] = None, | ||
| _headers: Optional[Dict[StrictStr, Any]] = None, | ||
| _host_index: Annotated[StrictInt, Field(ge=0, le={{#servers.size}}{{servers.size}}{{/servers.size}}{{^servers.size}}1{{/servers.size}})] = 0, | ||
| ) |
There was a problem hiding this comment.
Is this indentation on the last line suppose to be there?
There was a problem hiding this comment.
Ohhhh, yes!
This is a mustache partial template. It will be templated inside another template, and mustache does not seem to have good whitespace control for that. And you can surely imagine how this plays nicely with python. XD
There was a problem hiding this comment.
|
This new version of the container uses pydantic to validate http responses. I believe this will lead to a whole lot of new test failures. I'm not sure we are ready for this. |
mdellweg
force-pushed
the
update_container
branch
2 times, most recently
from
fb5c54d to
23b017f
Compare
mdellweg
force-pushed
the
update_container
branch
from
23b017f to
c07ca58
Compare
|
If we merge this now, i think it will be picked up once pulpcore main bumped it's version to 3.70.0.dev. And we have time til mid January to fix stuff, right? |
I hope so |
[noissue]
mdellweg
force-pushed
the
update_container
branch
from
c07ca58 to
55c7e88
Compare
There was a problem hiding this comment.
From what I remember (in the context I worked), most of, if not all, the mustache stuff was just a copy over from the generator. Is that the case here? Back then I though this was a bit confusing that we were shadowing files without any changes. Also it was not so straightfoward to spot what we are actually overriding or not.
If you copy the files into the original openapi-generator repository, you can diff that. I even committed them there to reabase onto the newer tag and copied the result back. All three of those files carry pulp specific changes. |
No description provided.