Open-api: update prefix param description #9870
Conversation
ajantha-bhat
changed the title
| @@ -1444,7 +1444,7 @@ components: | |||
| schema: | |||
| type: string | |||
| required: true | |||
| description: An optional prefix in the path | |||
| description: Prefix in the path | |||
There was a problem hiding this comment.
It is actually a required field. Cannot be optional.
I tried changing the field to optional. I get validation error as "in: path" params must be required.
So updated the description.
There was a problem hiding this comment.
so should required be set to false then?
There was a problem hiding this comment.
I mean, if I set it to false (I tried changing the field to optional), validator will fail. So, I assumed Paths must be required and changed the description.
There was a problem hiding this comment.
What's the error you're seeing?
The prefix is optional, but it depends on the definition. We can also update the default to be an empty string.
There was a problem hiding this comment.
If I change to required: false for prefix
Spec is invalid.
Issues:
components.parameters.For path parameter prefix the required value should be true
Execution failed for task ':iceberg-open-api:validateRESTCatalogSpec'.
> Validation failed.
There was a problem hiding this comment.
Thanks @Fokko for digging more on this. Their goal for publishing V4 is end of 2024.
So, As of now (since spec is based on v3), it is a required field. So, lets update the description and merge this as it is confusing.
There was a problem hiding this comment.
personally I would just leave things as they are and not change anything at this point
There was a problem hiding this comment.
So, As of now (since spec is based on v3), it is a required field. So, lets update the description and merge this as it is confusing.
I'm also in favor of leaving it like this. It is just yet another thing we cannot express in the open-API specification, but the intent is that the prefix is optional.
There was a problem hiding this comment.
I agree, let's keep as it is for now. As we have the new REST spec proposal, we can add a topic related to that.
ajantha-bhat
requested review from
Fokko,
nastra,
rdblue,
danielcweeks and
jackye1995
|
ping. |
|
cc: @jbonofre |
|
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the [email protected] list. Thank you for your contributions. |
|
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time. |
No description provided.