Skip to content

Bring the preview GH action into the product #11780

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

Open
humitos opened this issue Nov 19, 2024 · 3 comments
Open

Bring the preview GH action into the product #11780

humitos opened this issue Nov 19, 2024 · 3 comments
Assignees
Labels
Feature New feature Needed: design decision A core team decision is required

Comments

@humitos
Copy link
Member

humitos commented Nov 19, 2024

We discussed about migrating the preview GH action into the product itself. The user should be able to mark Add link to preview in the pull request with comment and description as options.

I think we will need to use use the SocialAccount from one of the owners to put the link in the pull request as a comment or editing the description because we don't have a GitHub App for Read the Docs. We will need to research a little more here.

Note there are reason why most of these bots use a new comment, so we should probably use that approach only. Example, the content added to the description ends up in the commit message --which is something we don't want.

Sentry comment's example: #11765 (comment)

@humitos humitos added the Feature New feature label Nov 19, 2024
@github-project-automation github-project-automation bot moved this to Planned in 📍Roadmap Nov 19, 2024
@agjohnson agjohnson added the Needed: design decision A core team decision is required label Dec 3, 2024
@agjohnson
Copy link
Contributor

This would be good to start to figure out. Some things to explore:

  • Is GHA required?
  • Can we use a GHA in addition to our normal Oauth application, not changing authentication/authorization on our side yet?

@humitos
Copy link
Member Author

humitos commented Jan 14, 2025

There is some previous art regarding GHA in #8445 that I've done some time ago.

It would be good if we can prioritize this work since we have been jumping in/out into this work a few times already and we definitely need for a few different use cases, like less strict permissions.

@stsewd would you like to take on this initial research to answer the questions that Anthony raised? I know that django-allauth has changed in the last few months, so there may be something new we can take advantage of.

@stsewd
Copy link
Member

stsewd commented Jan 22, 2025

I think we should migrate to GH actions, I found a way that works without having two login options. The main idea is:

  • A new user signs up using our new github app, nothing to do here.
  • An existing user tries to sign in:
    • The user is redirected to our new app so it can be approved (we should warn users that we are migrating to GH app and they will be required to grant access to this new app).
    • We detect that this user already has an existing account with our old app.
    • Instead of creating a new account, we just link the new social account to the existing user, and we log the user in.

That's the basic idea on how we can achieve the migration with just one login page. The other step is to get users to install the app into the repositories they have imported, that requires a little more of user interaction, but we can just link the users to our app with a list of repositories pre-selected so they can approve access, this would be something like this:

  • The existing user is signed in, and his account has two social apps attached (the old and the new github integration).
  • We know the repos that are imported under that account, we can create a single link for them to approve the instalation of the new app into those same repos (https://docs.github.com/en/apps/creating-github-apps/about-creating-github-apps/migrating-oauth-apps-to-github-apps#prompt-users-to-install-your-github-app).
  • We need to change our remote repositories to have a reference to the installation, with that the RTD project will always be in sync with the repo (we have global webhooks now!)
  • Once the user has migrated all of his repos to the new app (aka installed the app into the repos), we can disconnect him from the old GH oauth app, and also ask them to revoke access to that app from GH (as the GH guide suggests).

I already did a PoC with allauth, and it's possible to have two github apps running at the same time, and it's also possible to connect the new app to existing accounts on the fly (#11942).

Once a project has been migrated to use the new gh app, it will be easier to integrate more features easily, like leaving a comment with a link to the PR preview, cloning private repos without having to worry about SSH keys, keep projects always in sync with repos, etc.

@stsewd stsewd moved this from In progress to Needs review in 📍Roadmap Jan 22, 2025
@stsewd stsewd moved this from Needs review to Planned in 📍Roadmap Feb 24, 2025
trallard added a commit to pydata/pydata-sphinx-theme that referenced this issue Mar 25, 2025
This PR _should_ be the last of a series of improvements to our CI:

1. We have been having intermittent failures on our `docs-check`
workflow when trying to check links for `gnu` such as:

	```

/home/runner/work/pydata-sphinx-theme/pydata-sphinx-theme/docs/community/topics/i18n.rst:112:
WARNING: broken link:
https://www.gnu.org/software/gettext/manual/gettext.html#Fuzzy-Entries
(HTTPSConnectionPool(host='www.gnu.org', port=443): Max retries exceeded
with url: /software/gettext/manual/gettext.html (Caused by
NewConnectionError('<urllib3.connection.HTTPSConnection object at
0x7f6d203efb10>: Failed to establish a new connection: [Errno 101]
Network is unreachable')))
	```

	This PR tries to fix the timeout/retry issues.

2. Remove the RTD preview action that adds the link to the PR comment,
as it has been failing despite having the correct permissions (and it
seems RTD folks plan to move this to RTD directly
readthedocs/readthedocs.org#11780)
3. Fixes the last trigger-related issues after changing the `coverage`
workflow to `workflow_call` so we get our coverage comments again (I
have a PR in my repo with this working again
trallard#2)

---------

Co-authored-by: Daniel McCloy <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature Needed: design decision A core team decision is required
Projects
Status: Planned
Development

No branches or pull requests

3 participants