Skip to content

Specifying Query Parameters for call: HTTP #910

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

Closed
matthias-pichler opened this issue Jun 27, 2024 · 6 comments · Fixed by #956
Closed

Specifying Query Parameters for call: HTTP #910

matthias-pichler opened this issue Jun 27, 2024 · 6 comments · Fixed by #956
Labels
area: spec Changes in the Specification change: feature New feature or request. Impacts in a minor version change
Milestone

Comments

@matthias-pichler
Copy link
Collaborator

matthias-pichler commented Jun 27, 2024

What would you like to be added:

Would it make sense to add a structured way to add query parameters to uris when using call: http?

document: {}
do:
  - listPets:
      call: http
      with:
        method: get
        endpoint: 
          uri: https://petstore.swagger.io/v2/pets
        search:
          page[size]: ${ .pageSize }
          str: "Some string with characters & stuff that would need to be uri percent encoded! 👍 "

We could use the name "search", "query", "searchParameters", "queryparameters" or "parameters"

Why is this needed:

right now it is somewhat painful to specify these things using a runtime expression because we'd need to use the @uri format. And @uri also only encodes interpolated strings so the keys would have to manually encoded

document: {}
do:
  - listPets:
      call: http
      with:
        method: get
        endpoint: 
          uri: ${ @uri "https://petstore.swagger.io/v2/pets?page%5Bsize%5D=\(.pageSize)&str=\(\"Some string with characters & stuff that would need to be uri percent encoded! 👍 \")" }
@cdavernas
Copy link
Member

cdavernas commented Jun 27, 2024

@matthias-pichler-warrify you can also use top level args with { }, as demonstrated by https://github.com/serverlessworkflow/specification/blob/main/dsl-reference.md#http-call. I cannot find the place where this is documented, though I'd swear I wrote it.

As for uri encoding, I always assumed that was the responsibility of runtimes.

@matthias-pichler
Copy link
Collaborator Author

I only saw arguments under call: openapi but didn't find anything for call: http

@cdavernas cdavernas added change: feature New feature or request. Impacts in a minor version change area: spec Changes in the Specification labels Jun 30, 2024
@cdavernas cdavernas added this to the v1.0.0-alpha3 milestone Jun 30, 2024
@matthias-pichler
Copy link
Collaborator Author

matthias-pichler commented Jul 15, 2024

@cdavernas

As for uri encoding, I always assumed that was the responsibility of runtimes.

In principle I am with you on this one but I am thinking of a case where the query parameter keys would also need encoding (e.g. page[size]). In this case the author would have to properly encode the square brackets when they include them in the uri.

I am also wondering how to elegantly handle array valued query params as there are different representations:

  • repetition
  • empty square brackets
  • comma separated

Which could be handled with an appropriate field

@cdavernas
Copy link
Member

cdavernas commented Jul 15, 2024

@matthias-pichler-warrify fair enough, you make a very good point!

What about using query or queryParameters? Amongst your proposals, those are the terms that best represent imho the parameters we are trying to describe, though that's obviously opiniated and tainted by the techs I use.

On another note, I believe it would be awesome if the PR addressing the present issue would include documenting the ease of use feature I described in my last comment.

Wdyt?

@matthias-pichler
Copy link
Collaborator Author

On another note, I believe it would be awesome if the PR addressing the present issue would include documenting the ease of use feature I described in my last comment.

yeah the top level interpolation caught me by surprise when I first saw it 😅 Questions here:

  • What if the top level attribute does not exist? empty result or expression error?
  • can one use pet.id if pet is top level?
  • what types are allowed and how are they serialized? specifically objects and arrays

@cdavernas
Copy link
Member

cdavernas commented Jul 23, 2024

What if the top level attribute does not exist? empty result or expression error?

In Synapse, if an expression fails to evaluate, we simply leave it as is. I'm however personally open to any and all suggestions!

can one use pet.id if pet is top level?

That's a very good and very subtle question. I would probably tend to say no, as if need be, one could always use the expression language interpolation feature if complex operations are required. I'm sure it would however be a great quality of life improvement, so we should probably restrict that feature to variable access, no matter the depth.

what types are allowed and how are they serialized? specifically objects and arrays

My take is that only primitive types should be allowed. In Synapse, we transform the result of the evaluation to a string, no matter the original type, so using complex objects or array would produce their string equivalency (which in .NET would be something like NameOfTheComplexObjectType)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: spec Changes in the Specification change: feature New feature or request. Impacts in a minor version change
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants