-
Notifications
You must be signed in to change notification settings - Fork 62
📖 (docs) draft catalogd docs with new metas endpoint #1841
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
Conversation
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1841 +/- ##
=======================================
Coverage 69.04% 69.04%
=======================================
Files 65 65
Lines 5263 5263
=======================================
Hits 3634 3634
Misses 1396 1396
Partials 233 233
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added a few comments, but /lgtm
@@ -13,6 +18,21 @@ As an example, to access the full FBC via the v1 API endpoint (indicated by path | |||
|
|||
the URL to access the service would be `https://catalogd-service.olmv1-system.svc/catalogs/operatorhubio/api/v1/all` | |||
|
|||
2. `api/v1/metas` endpoint that allows clients to retrive filtered portions of the FBC. | |||
|
|||
The metas endpoint accepts additional parameters following the pattern `/api/v1/metas?<parameters>`, and accepts parameters that corresponds to any combination of the fields of the [FBC Meta structure](https://olm.operatorframework.io/docs/reference/file-based-catalogs/#schema) (except for `Blob`). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
additional parameters following the pattern
I think we could just say 'query parameters'?
Also, accepts parameters
repeats right after comma. Maybe just: The metas endpoint accepts query parameters that correspond to...
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to any combination of the fields ... (except for
Blob
)
maybe being more explicit here and for listing for example the elements of the Meta
schema might make sense?
Another thing is, what does Blob
refer to? Reading the documentation, there is this for example:
Each operator package in a catalog requires exactly one olm.package blob, at least one olm.channel blob, and one or more olm.bundle blobs.
but those would be valid query params, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea it's a little confusing. The Blob
I was trying to highlight isn't a valid query parameter is this Blob.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, there's a lot of blobs ;)
docs/howto/catalog-queries.md
Outdated
@@ -12,6 +12,9 @@ Then you can query the catalog by using `curl` commands and the `jq` CLI tool to | |||
By default, Catalogd is installed with TLS enabled for the catalog webserver. | |||
The following examples will show this default behavior, but for simplicity's sake will ignore TLS verification in the curl commands using the `-k` flag. | |||
|
|||
!!! note |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this doesn't render correctly, I think the syntax is [!NOTE], or maybe [!IMPORTANT]?
Similar case for https://github.com/operator-framework/operator-controller/pull/1841/files#diff-6295b5bd58ccf02e628fe42adbc43f32b9df3f3ad4ab61ac813366162d65a490R54 and https://github.com/operator-framework/operator-controller/pull/1841/files#diff-944a5a8340da19a891934706c3a6a9f5eeded7eacdce3c107edd1f9dfdd7f173R36
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved in #1841 (comment)
Not sure if I clicked any button but I did not dismiss any reviews 👀 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For net-new functionality, the process default is to add a new "drafts" doc.
Also, this feature is still feature gated and off by default. Official docs should probably not include information about feature gated features, except potentially in a separate doc that lists all of the feature gates and describes what they do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It totally slipped my mind that this feature is behind a feature gate and isn't GA yet. I've moved the docs that need updating with the new endpoint (when feature gate is enabled) to the /docs/drafts
folder as per our discussion in slack. PTAL
Rebased and pushed cc: @joelanford @grokspawn @azych |
|
||
Responses are encoded as a [JSON Lines](https://jsonlines.org/) stream of [File-Based Catalog](https://olm.operatorframework.io/docs/reference/file-based-catalogs) (FBC) [Meta](https://olm.operatorframework.io/docs/reference/file-based-catalogs/#schema) objects delimited by newlines. | ||
|
||
??? example "Example JSON-encoded FBC snippet" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here, this doesn't display correctly, together with the two ``` blocks below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fine with our rendering system ... just not the github review front-end.
This doc includes both sequences: https://operator-framework.github.io/operator-controller/api-reference/catalogd-webserver/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good to know, thanks. @anik120 feel free to resolve, as I can't do it with my access level :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
apart from multiple places where formatting doesn't display correctly (!!!, ???, ```), the actual documentation looks good to me
/lgtm
I see multiple instances of I'm merging this PR with the lgtm I have since it's in |
Actually, looks like @grokspawn has a requested review, going to wait for a bit |
|
||
For more examples of valid queries that can be made to the `api/v1/metas` service endpoint, please see [Catalog Queries](../howto/catalog-queries.md). | ||
|
||
!!! note |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't have a problem here. The !!! note
notation is supported by the mkdocs pipeline.
For e.g. see existing rendered docs here, immediately before the section 3 header: "Granting User Access to API Resources".
!!! important | ||
Currently, OLM 1.0 does not support the installation of extensions that use webhooks or that target a single or specified set of namespaces. | ||
|
||
3. Return list of packages where the channel head version use `olm.csv.metadata` format, support `AllNamespaces` install mode and do not use webhooks: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
3. Return list of packages where the channel head version use `olm.csv.metadata` format, support `AllNamespaces` install mode and do not use webhooks: | |
3. Return list of packages which support `AllNamespaces` install mode, do not use webhooks, and where the channel head version uses `olm.csv.metadata` format: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Closes operator-framework#1782 Signed-off-by: Anik Bhattacharjee <[email protected]>
New changes are detected. LGTM label has been removed. |
Closes #1782
Description
Reviewer Checklist