Skip to content

CI Handle Circle CI REST API response #25340

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

jjerphan
Copy link
Member

@jjerphan jjerphan commented Jan 9, 2023

Reference Issues/PRs

Follow-up of #25338.

What does this implement/fix? Explain your changes.

Circle CI REST API return HTTP response with 202 status code even when the POST requests fail.

This proposes adding some handling so that errors are reported on GitHub.

Any other comments?

Co-authored-by: Olivier Grisel olivier.grisel@ensta.org

Follow-up of scikit-learn#25338.

Circle CI REST API return HTTP response with 202 status code
even when the POST requests fail.

This proposes adding some handling so that errors are reported on
GitHub.

Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
@jjerphan jjerphan added Quick Review For PRs that are quick to review Waiting for Second Reviewer First reviewer is done, need a second one! labels Jan 10, 2023
@lesteve
Copy link
Member

lesteve commented Jan 19, 2023

Honestly I would be in favour of moving back the doc builds to CircleCI. This would simplify the setup quite a bit ... is there a good reason not to do this?

This would look (like in the goold old days 😉):

  • build doc and upload browsable artifacts on CircleCI

The way it looks right now:

  • build doc on github actions and upload artifact
  • kick CircleCI build with curl from github action (needs an noop build on CircleCI for this)
  • Circle CI downloads artifacts from GHA and uploads its to its artifact which can be browsed

Alternative option since we are not using CircleCI for linux-arm64 now would be to use Netlify to have browsable artifacts, see https://github.com/INRIA/scikit-learn-mooc/blob/main/.github/workflows/jupyter-book-pr-preview.yml where I did that for the MOOC.

There are some limitation on Netlify but we can ask for an open-source plan if we think we need it.

Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
@jjerphan
Copy link
Member Author

Honestly I would be in favour of moving back the doc builds to CircleCI. This would simplify the setup quite a bit ... is there a good reason not to do this?

I think so. I trust you regarding taking decisions to adapt this setup for the best (i.e. probably the simplest).

@thomasjpfan
Copy link
Member

@lesteve I am okay with moving back to CircleCI. You are welcome to ping me on a PR that makes that move.

I had a sketch of a way to host PR docs without CircleCI using GitHub pages (on a separate repo), that should be secure. But I did not want to invest time in it, because GitHub plans to do it in the future: community/community#7730 (comment).

@lesteve lesteve mentioned this pull request Jan 24, 2023
3 tasks
@jjerphan jjerphan deleted the ci/handle-circle-ci-rest-api-response branch January 24, 2023 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build / CI No Changelog Needed Quick Review For PRs that are quick to review Waiting for Second Reviewer First reviewer is done, need a second one!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants