Skip to content
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

[DOC] Auto-generate openapi.json #782

Merged
merged 24 commits into from
Aug 7, 2024

Conversation

pavithraes
Copy link
Member

@pavithraes pavithraes commented Mar 14, 2024

Ref: #766

Description

This pull request:

  • Adds a script and action to auto-update the openapi.json static file. This file is used to generate the REST API docs.
  • Adds conda-store.sqlite to .gitignore

Pull request checklist

  • Did you test this change locally?
  • Did you update the documentation (if required)?
  • Did you add/update relevant tests for this change (if required)?

Additional information

  • While this works, there is likely a cleaner/minimal way to do this. Please let me know if you ideas to simplify this!
  • Please make sure to squash-merge this PR.

How to test

I've tested it on my fork: pavithraes#4

Signed-off-by: Pavithra Eswaramoorthy <pavithraes@outlook.com>
Signed-off-by: Pavithra Eswaramoorthy <pavithraes@outlook.com>
Signed-off-by: Pavithra Eswaramoorthy <pavithraes@outlook.com>
Signed-off-by: Pavithra Eswaramoorthy <pavithraes@outlook.com>
Signed-off-by: Pavithra Eswaramoorthy <pavithraes@outlook.com>
Signed-off-by: Pavithra Eswaramoorthy <pavithraes@outlook.com>
Signed-off-by: Pavithra Eswaramoorthy <pavithraes@outlook.com>
Signed-off-by: Pavithra Eswaramoorthy <pavithraes@outlook.com>
Signed-off-by: Pavithra Eswaramoorthy <pavithraes@outlook.com>
Signed-off-by: Pavithra Eswaramoorthy <pavithraes@outlook.com>
Signed-off-by: Pavithra Eswaramoorthy <pavithraes@outlook.com>
Signed-off-by: Pavithra Eswaramoorthy <pavithraes@outlook.com>
@pavithraes pavithraes added area: documentation 📖 Improvements or additions to documentation needs: review 👀 labels Mar 14, 2024
@pavithraes pavithraes requested a review from dcmcand March 14, 2024 19:17
Copy link

netlify bot commented Mar 14, 2024

Deploy Preview for kaleidoscopic-dango-0cf31d ready!

Name Link
🔨 Latest commit 269d2cc
🔍 Latest deploy log https://app.netlify.com/sites/kaleidoscopic-dango-0cf31d/deploys/65f34d4bf9e3b40008cb07ff
😎 Deploy Preview https://deploy-preview-782--kaleidoscopic-dango-0cf31d.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

Copy link

netlify bot commented Mar 14, 2024

Deploy Preview for conda-store canceled.

Name Link
🔨 Latest commit ad208f8
🔍 Latest deploy log https://app.netlify.com/sites/conda-store/deploys/66b248c395cb7b0008c0543a

@pavithraes pavithraes mentioned this pull request Mar 14, 2024
3 tasks
Copy link
Contributor

@peytondmurray peytondmurray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick question: this will make a PR against the branch that someone is trying to merge, right? So the workflow looks like this:

  1. I make a PR
  2. This action runs, updating openapi.json. A PR against my branch is opened.
  3. I merge the PR to my branch
  4. Then I merge my branch to main

Is that the workflow?


If I've got that workflow right, I'd vote to have a different workflow similar to how pre-commit.ci works:

  1. I make a PR
  2. This action runs, updating openapi.json. A commit to my branch is made.
  3. I merge my branch to main

But that's just my preference.

.github/workflows/generate_api_docs.yaml Outdated Show resolved Hide resolved
docusaurus-docs/scripts/generate_openapi_json.py Outdated Show resolved Hide resolved
.github/workflows/generate_api_docs.yaml Outdated Show resolved Hide resolved
@pavithraes
Copy link
Member Author

pavithraes commented Jun 4, 2024

Thanks for the review!

Quick question: this will make a PR against the branch that someone is trying to merge, right?

@peytondmurray No, this workflow opens a PR against main when a conda-store release is made. Since the docs are deployed separately, this helps us keep the API ref up-to-date with each released version.

Signed-off-by: Pavithra Eswaramoorthy <pavithraes@outlook.com>
pavithraes and others added 2 commits June 4, 2024 21:31
@peytondmurray
Copy link
Contributor

@peytondmurray No, this workflow opens a PR against main when a conda-store release is made. Since the docs are deployed separately, this helps us keep the API ref up-to-date with each released version.

So my one problem with this is that doc versions that track the main branch will have API docs that are out of date. Are we publishing docs for the main branch? Do we even need to worry about this?

@pavithraes
Copy link
Member Author

That's fair. The docs are currently only built against main.

I didn't want to generate the file on every merge to main because not all PRs affect the API.

I think we can re-generate this on updates to certain files. I'm not sure which specific files to track here, but I can update the PR to regenerate openapi.json on updates to anything within conda_store_server/ for now.

Signed-off-by: Pavithra Eswaramoorthy <pavithraes@outlook.com>
Signed-off-by: Pavithra Eswaramoorthy <pavithraes@outlook.com>
Signed-off-by: Pavithra Eswaramoorthy <pavithraes@outlook.com>
@peytondmurray
Copy link
Contributor

Failing unit tests look like an unspecified conda error:

ERROR tests/test_actions.py::test_api_get_build_installer[conda_prefix0] - conda_lock._vendor.poetry.utils._compat.CalledProcessError: Command '['mamba', 'env', 'create', '-p', '/tmp/pytest-of-runner/pytest-0/test_api_get_build_installer_c0/test-prefix', '-f', '/tmp/tmpf5eqekfj/environment.yaml']' returned non-zero exit status 1.
ERROR tests/test_actions.py::test_api_get_build_installer[conda_prefix1] - conda_lock._vendor.poetry.utils._compat.CalledProcessError: Command '['mamba', 'env', 'create', '-p', '/tmp/pytest-of-runner/pytest-0/test_api_get_build_installer_c1/test-prefix', '-f', '/tmp/tmpc1ja3ttd/environment.yaml']' returned non-zero exit status 1.

I'll run these again to see if they're just some flaky network issue or something real. In any case, I'll add these to the meta testing issue so we can mock them out, because we're currently calling out to conda/mamba here.


Separately, after a discussion with @pavithraes this PR will be modified so that autogenerated API docs are committed directly to the PR branch (as pre-commit.ci changes are) to avoid generating separate PRs to merge. We'll merge this PR once that small change is made.

@peytondmurray peytondmurray added the needs: discussion 💬 This item needs team-level discussion before scoping label Jul 23, 2024
Signed-off-by: Pavithra Eswaramoorthy <pavithraes@outlook.com>
Signed-off-by: Pavithra Eswaramoorthy <pavithraes@outlook.com>
@pavithraes
Copy link
Member Author

@peytondmurray - this PR will now add a new commit to PRs that touch the conda-store-server files. Ready for another review :)

Copy link
Contributor

@peytondmurray peytondmurray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing, thank you!

@peytondmurray peytondmurray merged commit b196477 into conda-incubator:main Aug 7, 2024
20 of 29 checks passed
@pavithraes pavithraes deleted the gen-openapi-json branch September 16, 2024 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: documentation 📖 Improvements or additions to documentation needs: discussion 💬 This item needs team-level discussion before scoping needs: review 👀
Projects
Status: Done 💪🏾
Development

Successfully merging this pull request may close these issues.

2 participants