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.

Sign up for GitHub

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

ci: Move static builds to d2l.dev bucket #3898

Merged
merged 3 commits into from
Aug 24, 2023
Merged

ci: Move static builds to d2l.dev bucket #3898

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5a04d6b
ci: Move static builds to d2l.dev bucket
eKoopmans Aug 17, 2023
9cec0a5
Move to live.d2l.dev
eKoopmans Aug 24, 2023
a586f8d
Use github.repository variable
eKoopmans Aug 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,14 @@ jobs:
aws-access-key-id: ${{ secrets.AWS_ACCESS_KEY_ID }}
aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
aws-session-token: ${{ secrets.AWS_SESSION_TOKEN }}
role-to-assume: arn:aws:iam::037018655140:role/brightspace-ui-core.d2l.dev-githubactions-access
role-to-assume: arn:aws:iam::022062736489:role/r+BrightspaceUI+core+repo
role-duration-seconds: 3600
aws-region: ca-central-1

- name: Publish
uses: BrightspaceUI/actions/publish-to-s3@main
with:
bucket-path: s3://brightspace-ui-core.d2l.dev/branches/pr-${{ github.event.number }}
bucket-path: s3://d2l.dev/pr-previews/brightspace-ui-core/pr-${{ github.event.number }}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think I pictured this as:
bucket-path: s3://d2l.dev/pr-previews/BrightspaceUI/core/pr-${{ github.event.number }}

But I guess it doesn't really matter, as long as there are no collisions in what people choose 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

If we did do org/repo, we could actually add a special thing to repo_settings so someone could do:

pr_previews: true

and under the hood it would setup the d2ldev piece. But that doesn't work with the "main" site piece.

Just spit-balling here - what are your thoughts on this:

  • All previews go into d2l.dev/pr-previews
    • pr-previews/main/<org>/<repo>/ for the main site that doesn't expire
    • pr-previews/prs/<org>/<repo>/pr-<#> for the PR sites
  • The expiry prefix is changed to pr-previews/prs
  • For now, someone sets this in repo-settings (need to confirm this doesn't blow up): 
    d2ldev:
  s3:
    - subdomain: pr-previews
      path: main/<org>/<repo>
    - subdomain: pr-previews
      path: prs/<org>/<repo>
    • But we eventually switch this to pr_previews: true and set it for them
  • If someone wants a fancy path like brightspace-ui-core.d2l.dev, they can set it up in d2l.dev. But they don't actually have to set up anything in d2l.dev if they're fine with https://pr-previews.d2l.dev/prs/BrightspaceUI/core/pr-${{ github.event.number }}/ and https://pr-previews.d2l.dev/main/BrightspaceUI/core/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm open to that, except that I think the phrase "PR preview" doesn't apply any more to the "main" static builds.

What about calling them all repo-demos or code-demos?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah good with either of those! Or even demos could work, if we want to try to make the default urls as easy to remember as possible (maybe not useful with url autocomplete lol)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay I might be back to being against this, mostly because of how the URLs would work:

I don't like that folder restriction, but maybe it's fine...

Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't the pr and main need to be at the beginning to be able to set the retention properly, since we can't use wildcards in the prefix? Like https://demos.d2l.dev/main/BrightspaceUI/core/ (or I think we could get away with https://demos.d2l.dev/BrightspaceUI/core/) and https://demos.d2l.dev/prs/BrightspaceUI/core/

I guess we could map it in some way with the routes, but this way maybe alleviates your concern cause there's no folder name they need to avoid. They publish their main site to d2ldev/demos/<org>/<repo> and their PR sites to d2ldev/demos/prs/<org>/<repo>/<whatever-they-want-really>-<pr_num>. And the lifecycle prefix is "/demos/prs/"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah the prefix would need to be there in the bucket, but like you said the routing doesn't need to match the bucket.

I like keeping the main demo link clean as https://demos.d2l.dev/BrightspaceUI/core/. You're right that sticking the prs prefix into the URL works, though it's a bit buried there...

We could also just do a completely different route and serve the prs/ bucket up at https://pr-previews.d2l.dev/BrightspaceUI/core/. That might be an anti-pattern though, d2l.dev is generally set up so the bucket prefix matches the subdomain.

So then - why not just two separate bucket prefixes? Keep pr-previews as it currently is, and just add one for demos? Is there any benefit to trying to merge them into one, apart from their content being similar? Keeping pr-previews clearly separate might also make the expiration rule more obvious.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright I thought on it a bit, I can get behind https://demos.d2l.dev/prs/BrightspaceUI/core/ - the URL for these aren't too important anyway, since you're only going to be visiting them as links from PRs. And for that same reason, having a whole subdomain dedicated to them isn't too useful.

Any other thoughts before I go configure that in d2l.dev?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm good with either of your last two comments - going with https://demos.d2l.dev/prs/BrightspaceUI/core/, or just making two bucket prefixes to keep things clear, since we could still wrap that in repo-settings behind a single command. I was trying to think of two buckets prefixes that worked nicely together lol... demo and demo-prs? pr-preview and repo-preview? preview and pr-preview?

publish-directory: ./build/

- name: Notify
Expand All @@ -84,7 +84,7 @@ jobs:

We've deployed an automatic preview for this PR - you can see your changes here:

| URL | https://pr-${{ github.event.number }}-brightspace-ui-core.d2l.dev/ |
| URL | https://pr-previews.d2l.dev/brightspace-ui-core/pr-${{ github.event.number }}/ |
|---|---|

> **Note**
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/release.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,12 @@ jobs:
aws-access-key-id: ${{ secrets.AWS_ACCESS_KEY_ID }}
aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
aws-session-token: ${{ secrets.AWS_SESSION_TOKEN }}
role-to-assume: arn:aws:iam::037018655140:role/brightspace-ui-core.d2l.dev-githubactions-access
role-to-assume: arn:aws:iam::022062736489:role/r+BrightspaceUI+core+repo
role-duration-seconds: 3600
aws-region: ca-central-1

- name: Publish
uses: BrightspaceUI/actions/publish-to-s3@main
with:
bucket-path: s3://brightspace-ui-core.d2l.dev/branches/main
bucket-path: s3://d2l.dev/brightspace-ui-core/main
publish-directory: ./build/