:book: add public api docs by everettraven · Pull Request #1331 · operator-framework/operator-controller

everettraven · 2024-10-03T14:02:25Z

Description

Adds documentation on the catalogd webserver that is considered as part of the public API
Adds documentation that explicitly calls out what our public API is
resolvesWhat is our public API? #1239

Reviewer Checklist

API Go Documentation
Tests: Unit Tests (and E2E Tests, if appropriate)
Comprehensive Commit Messages
Links to related GitHub Issue(s)

netlify · 2024-10-03T14:02:42Z

✅ Deploy Preview for olmv1 ready!

Name	Link
🔨 Latest commit	`c4e30d7`
🔍 Latest deploy log	https://app.netlify.com/sites/olmv1/deploys/6700535763043f000892b90f
😎 Deploy Preview	https://deploy-preview-1331--olmv1.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

codecov · 2024-10-03T14:16:34Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.37%. Comparing base (814bf63) to head (c4e30d7).
Report is 5 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #1331 +/- ##
=======================================
Coverage 76.37% 76.37% =======================================
Files 41 41 Lines 2438 2438 =======================================
Hits 1862 1862 Misses 402 402 Partials 174 174

Flag	Coverage Δ
e2e	`58.44% <ø> (ø)`
unit	`53.60% <ø> (-0.05%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

LalatenduMohanty

As per the last discussion HEAD https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods/HEAD is also a supported method for catalog webserver

LalatenduMohanty · 2024-10-03T18:33:30Z

Apart from my above comment i.e. #1331 (review) everything else looks good.

everettraven · 2024-10-03T18:46:18Z

@LalatenduMohanty addressed

michaelryanpeter

Small nits. I am going to look into getting some linting in the repo to address some of the consistency issues around terminology.

michaelryanpeter · 2024-10-03T19:36:41Z

+ - [operator-controller API reference](./api/operator-controller-api-reference.md)
+ - [catalogd API reference](./api/catalogd-api-reference.md)
+- `catalogd` webserver. For more information on what this includes, see the [catalogd webserver documentation](./api/catalogd-webserver.md)


Since this section is pretty short, I think you could create an Additional resources section at the end of the page for the links. Maybe after line 12?
## Additional resources - Operator Controller API reference - Catalogd API reference - Catalogd web server

Would you mind elaborating on what value this would bring to a reader over the current setup?

To me, the links are a little distracting where they are. The point you are trying to get across is to define the scope of the public API. I think that if you break out the links to the API references at the end of the page, it is a little tidier and your intended meaning is more direct.
That said, this is 100% a nit. Feel free to ignore it doesn't feel relevant to you.

As a heuristic for determining whether something should be an inline link vs an additional resource, I ask myself if the reader needs to stop and read the link before continuing.
If the answer is yes, then I use an inline link. Typically, this would be in the case of a prerequisite or context information that is vital to understanding a concept or completing a task.
If the reader can follow up on the information later or if the link is provided for helpful context, then I break out the link in an additional resources section.

As long as the content is OK - i'd like to get this merged in so we at least have it and can keep progressing towards our v1.0.0 release. We can always tweak this in the future!

Signed-off-by: everettraven <everettraven@gmail.com>

trgeiger · 2024-10-04T14:03:51Z

+# Catalogd web server
+
+[Catalogd](https://github.com/operator-framework/catalogd), the OLM v1 component for making catalog contents available on cluster, includes
+a web server that serves catalog contents to clients via a HTTP(S) endpoint.


This would be "an HTTP(s)" since HTTP is pronounced with a starting vowel sound. You determine the article via the acronym pronunciation.

Should be taken care of in 647a9f7

trgeiger · 2024-10-04T14:06:16Z

+
+### Compression Support
+
+The `catalogd` web server supports gzip compression of responses, which can significantly reduce associated network traffic. In order to signal to that the client handles compressed responses, the client must include `Accept-Encoding: gzip` as a header in the HTTP request.


"to signal that the client"

Should be taken care of in 647a9f7

michaelryanpeter · 2024-10-04T14:42:06Z

+1 to Tayler's comments. Otherwise, lgtm.

Signed-off-by: everettraven <everettraven@gmail.com>

Co-authored-by: Joe Lanford <joe.lanford@gmail.com>

everettraven requested a review from a team as a code owner October 3, 2024 14:02

LalatenduMohanty requested changes Oct 3, 2024
View reviewed changes

everettraven requested a review from LalatenduMohanty October 3, 2024 18:46

everettraven enabled auto-merge October 3, 2024 18:58

grokspawn reviewed Oct 3, 2024
View reviewed changes

Comment threaddocs/refs/api/catalogd-webserver.md Outdated

michaelryanpeter reviewed Oct 3, 2024
View reviewed changes

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 3, 2024

everettraven added 2 commits October 3, 2024 16:09

public api docs
3b934d9
Signed-off-by: everettraven <everettraven@gmail.com>

post rebase fixes
fc29827
Signed-off-by: everettraven <everettraven@gmail.com>

everettraven force-pushed the docs/public-api branch from de1767e to fc29827Compare October 3, 2024 20:12

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 3, 2024

trgeiger reviewed Oct 4, 2024
View reviewed changes

address nits
647a9f7
Signed-off-by: everettraven <everettraven@gmail.com>

joelanford reviewed Oct 4, 2024
View reviewed changes

Comment threaddocs/api-reference/catalogd-webserver.md Outdated

Update docs/api-reference/catalogd-webserver.md
c4e30d7
Co-authored-by: Joe Lanford <joe.lanford@gmail.com>

LalatenduMohanty approved these changes Oct 4, 2024
View reviewed changes

joelanford approved these changes Oct 4, 2024
View reviewed changes

everettraven added this pull request to the merge queue Oct 4, 2024

Merged via the queue into operator-framework:main with commit eb64d98Oct 4, 2024


		### Compression Support

		The `catalogd` web server supports gzip compression of responses, which can significantly reduce associated network traffic. In order to signal to that the client handles compressed responses, the client must include `Accept-Encoding: gzip` as a header in the HTTP request.

Uh oh!

Conversation

everettraven commented Oct 3, 2024

Description

Reviewer Checklist

Uh oh!

netlifyBot commented Oct 3, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for olmv1 ready!

Uh oh!

codecovBot commented Oct 3, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

LalatenduMohanty left a comment

Choose a reason for hiding this comment

Uh oh!

LalatenduMohanty commented Oct 3, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

everettraven commented Oct 3, 2024

Uh oh!

Uh oh!

michaelryanpeter left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

michaelryanpeterOct 3, 2024

Choose a reason for hiding this comment

Uh oh!

everettravenOct 3, 2024

Choose a reason for hiding this comment

Uh oh!

michaelryanpeterOct 3, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

michaelryanpeterOct 3, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

everettravenOct 4, 2024

Choose a reason for hiding this comment

Uh oh!

trgeigerOct 4, 2024

Choose a reason for hiding this comment

Uh oh!

everettravenOct 4, 2024

Choose a reason for hiding this comment

Uh oh!

trgeigerOct 4, 2024

Choose a reason for hiding this comment

Uh oh!

everettravenOct 4, 2024

Choose a reason for hiding this comment

Uh oh!

michaelryanpeter commented Oct 4, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

netlifyBot commented Oct 3, 2024•
edited
Loading

codecovBot commented Oct 3, 2024•
edited
Loading

LalatenduMohanty commented Oct 3, 2024•
edited
Loading

michaelryanpeterOct 3, 2024•
edited
Loading

michaelryanpeterOct 3, 2024•
edited
Loading