Update container #98
Update container #98
Conversation
mdellweg
force-pushed
the
update_container
branch
5 times, most recently
from
09f7286
to
a032ed7
Compare
This also allows to consume the *api.json from anywhere, as it's processed into patched-api.json prior to be used in the container. [noissue]
mdellweg
force-pushed
the
update_container
branch
2 times, most recently
from
10309ba
to
04312c5
Compare
[noissue]
mdellweg
force-pushed
the
update_container
branch
from
04312c5
to
b9099c8
Compare
| 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
|
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. |
No description provided.