Fixes #420 - Add REST invocation function definition based on OpenAPI Path Object #652
Conversation
ricardozanini
added
change: documentation
ricardozanini
requested review from
tsurdilo,
cdavernas and
antmendoza
as code owners
| @@ -90,6 +90,25 @@ | |||
| "name", | |||
| "operation" | |||
| ] | |||
| }, | |||
There was a problem hiding this comment.
i dont think this is right. we should find a way to restrict object type on the value of type property please.
There was a problem hiding this comment.
also description if object type should add something like "added only for rest type"
There was a problem hiding this comment.
I don't think so. The AuthRef object can be applied to any remote invocation such as gRPC, GraphQL, openapi, rest, odata, etc.
If we would make this change, I'd say to do it in another PR. There's no change here apart from the formatting and the default to openapi type.
There was a problem hiding this comment.
Resolved? I personally agree with @ricardozanini on this one!
| { | ||
| "type": "object", | ||
| "description": "OpenAPI Path Object definition", | ||
| "$comment": "https://spec.openapis.org/oas/v3.1.0#paths-object", |
There was a problem hiding this comment.
i dont think we use schema 7.x comments anywhere do we? maybe can just put in description and try to be version independent
There was a problem hiding this comment.
We have been using schema 7.x since the beginning, and I don't think it will break anything. $comment is pretty useful since SDKs/Tooling can use it to parse this data against the OpenAPI spec. So we don't need to replicate their structure here.
Additionally, schema 7.x is from March 2018.
Honestly, I don't see why we should not use $comment based on this tradeoff.
There was a problem hiding this comment.
@tsurdilo Any further toughts? If not, please resolve ;)
|
@cdavernas thanks! I'll rebase and deal with the comments today. |
ricardozanini
force-pushed
the
issue-420-2
branch
from
bf30a86 to
85b970a
Compare
|
@tsurdilo, I believe I've incorporated most of your comments. There's one left that I'll handle later. Can you take another look, please? |
|
looking |
|
Will do review today (one last time) if thats ok. |
|
@tsurdilo You had a chance to confirm this one? |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
@cdavernas I'm gonna rebase this PR. Do you see anything else we need before merging it? |
|
@ricardozanini nice, cheers mate! |
|
@cdavernas I'm gonna rebase and merge tomorrow. |
…n based on OpenAPI Path Item Signed-off-by: Ricardo Zanini <[email protected]>
Signed-off-by: Ricardo Zanini <[email protected]>
ricardozanini
force-pushed
the
issue-420-2
branch
from
85b970a to
bfa990d
Compare
|
@cdavernas I think we are ready, I already notified the community. I made one last change to rebase and reflect the naming convention PR. Mind taking a last look? |
Many thanks for submitting your Pull Request ❤️!
Please specify parts of this PR update:
Discussion or Issue link:
See #647
What this PR does / why we need it:
In our latest Community Meeting, we discussed the possibility of adding support for RESTful operation descriptors in the Function definitions based on OpenAPI Path Item object. Here's the PR. The former PR has been closed.
Special notes for reviewers:
The "Security Object" will be ignored by the specification since we already have the Auth Definition to fit this role. Two places to define the same information can be troublesome and error prune.
Additional information:
See reference: https://spec.openapis.org/oas/v3.1.0#paths-object