Re-Introduce: Update Site's REST Catalog Spec Reference to Bundled Polaris + Iceberg API YAML#953
Conversation
…+ Iceberg API YAML (apache#935)" This reverts commit 3685065.
HonahX
requested review from
MonkeyCanCode,
RussellSpitzer,
adutra,
ashvina,
collado-mike,
dennishuo,
dimas-b,
ebyhr,
eric-maynard,
flyrain,
jackye1995,
jbonofre,
snazy,
takidau and
vvcephei
as code owners
There was a problem hiding this comment.
This is generated incorrectly due to a wrong reference to IcebergErrorResponse.
There was a problem hiding this comment.
nit: "OpenAPI" is a standard for defining APIs. It is not an API that Polaris implements: Proposal: Polaris API specification.
We can mention following OpenAPI guidelines and including the Iceberg REST API in the description of the spec.
There was a problem hiding this comment.
How about Apache Polaris Catalog API Specification (I've updated to that)?
Do we want to also update
polaris/site/content/in-dev/unreleased/polaris-management-service.md
Lines 20 to 24 in 012bdcc
to something like Apache Polaris Management API Specification?
(The title won't matter much now as user will be redirected to swagger editor soon after clicking so they will have about 1 second to see the title)
There was a problem hiding this comment.
Would you mind making actual API spec changes in a separate PR? This PR's title indicates it's a "doc" change :)
There was a problem hiding this comment.
Removed the spec changes. Currently the generated yaml will throw some error at the top but still render the all the endpoints definition. I will opened a new PR for that fix : )
There was a problem hiding this comment.
Do you mean #957? I think we can merge it first.
There was a problem hiding this comment.
Yes! I will mark this one as depends on #957
HonahX
force-pushed
the
honahx_update_site_2
branch
from
563b0ba to
da79d77
Compare
flyrain
left a comment
flyrain
left a comment
There was a problem hiding this comment.
LGTM with a minor comment.
| title: 'Apache Iceberg OpenAPI' | ||
| linkTitle: 'Iceberg OpenAPI' | ||
| title: 'Apache Polaris Catalog API Specification' | ||
| linkTitle: 'Catalog API Spec' |
There was a problem hiding this comment.
Nit: As we are doing swagger editor redirection, the title won't share in the page. I think we could just remove either title or linkTitle
There was a problem hiding this comment.
Good point! I tried and found that removing the title can make it cleaner. We will still need linkTitle since that appears in the side bar
3 participants
Depends on #957
This PR re-introduce the #935, which is first merged and later reverted (#942) when trying to solve the site issue: #941
The site issue is now fixed with #950 so we are now ready to move forward with the site change.
Updated the website's reference to generated/bundled-polaris-catalog-service.yaml instead of rest-catalog-open-api.yaml. This change ensures that Polaris-native APIs and models can be removed from rest-catalog-open-api.yaml in the next step, allowing it to align with the released version.
cc @flyrain @jackye1995