Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Honor @http annotation for query parameters #197

Conversation

jhernand
Copy link
Collaborator

Currently the @http annotation is ignored for query parameters. We are using @json instead. That is a bug, and this patch fixes it.

jhernand added a commit to jhernand/ocm-api-model that referenced this pull request Jun 22, 2023
Currently we are using the `@json` annotation to force the name of the
dry run query parameter to `dryRun`. But this is incorrect, and only
works because of a bug in the metamodel that is going to be fixed. This
patch replaces `@json` with `@http` which will work after the bug is
fixed.

Related: openshift-online/ocm-api-metamodel#197
Signed-off-by: Juan Hernandez <juan.hernandez@redhat.com>
jhernand added a commit to jhernand/ocm-api-model that referenced this pull request Jun 22, 2023
Currently we are using the `@json` annotation to force the name of the
dry run query parameter to `dryRun`. But this is incorrect, and only
works because of a bug in the metamodel that is going to be fixed. This
patch replaces `@json` with `@http` which will work after the bug is
fixed.

Related: openshift-online/ocm-api-metamodel#197
Signed-off-by: Juan Hernandez <juan.hernandez@redhat.com>
@jhernand jhernand force-pushed the honor_http_annotation_in_query_parameters branch from 9d90305 to 5cf844b Compare June 22, 2023 16:04
Currently the `@http` annotation is ignored for query parameters. We are
using `@json` instead. That is a bug, and this patch fixes it.

Signed-off-by: Juan Hernandez <juan.hernandez@redhat.com>
@jhernand jhernand force-pushed the honor_http_annotation_in_query_parameters branch from 5cf844b to 4998940 Compare June 22, 2023 18:53
@jhernand jhernand merged commit 3927e39 into openshift-online:main Jun 22, 2023
@jhernand jhernand deleted the honor_http_annotation_in_query_parameters branch June 22, 2023 18:59
@cben
Copy link

cben commented Jun 22, 2023

Given

-	in fetchLabels Boolean
+	@http(name = "fetchLabels")
+	in FetchLabels Boolean
  • this fixed openapi:
@@        "parameters": [
-            "name": "fetchlabels_labels",
+            "name": "fetchLabels",
  • and the method name & I think the actually sent param in generated Go:
-// FetchlabelsLabels sets the value of the 'fetchlabels_labels' parameter.
-func (r *SubscriptionsListRequest) FetchlabelsLabels(value bool) *SubscriptionsListRequest {
+// FetchLabels sets the value of the 'fetch_labels' parameter.
+func (r *SubscriptionsListRequest) FetchLabels(value bool) *SubscriptionsListRequest {
...
-		helpers.AddValue(&query, "fetchlabels_labels", *r.fetchlabelsLabels)
+		helpers.AddValue(&query, "fetchLabels", *r.fetchLabels)
  • tiny nit: the parameter name in the comment remained snake_case. I think the SDK method docs should refer to things as actually sent on the wire, so ideally want:
// FetchLabels sets the value of the 'fetchLabels' parameter.

cben added a commit to cben/ocm-api-model that referenced this pull request Jun 22, 2023
Requires metamodel >= 0.0.59 to respect the annotation
(openshift-online/ocm-api-metamodel#197)

However, generated names before were wrong anyway.
cben added a commit to cben/ocm-api-model that referenced this pull request Jun 23, 2023
Requires metamodel >= 0.0.59 to respect the annotation
(openshift-online/ocm-api-metamodel#197)

However, generated names before were wrong anyway.
tzvatot pushed a commit to tzvatot/ocm-api-model that referenced this pull request Jul 24, 2023
Requires metamodel >= 0.0.59 to respect the annotation
(openshift-online/ocm-api-metamodel#197)

However, generated names before were wrong anyway.
tzvatot pushed a commit to tzvatot/ocm-api-model that referenced this pull request Jul 24, 2023
Currently we are using the `@json` annotation to force the name of the
dry run query parameter to `dryRun`. But this is incorrect, and only
works because of a bug in the metamodel that is going to be fixed. This
patch replaces `@json` with `@http` which will work after the bug is
fixed.

Related: openshift-online/ocm-api-metamodel#197
Signed-off-by: Juan Hernandez <juan.hernandez@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants