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

Reroute home page to FEM #372

Merged
merged 8 commits into from
Sep 16, 2024
Merged

Reroute home page to FEM #372

merged 8 commits into from
Sep 16, 2024

Conversation

zwolf
Copy link
Member

@zwolf zwolf commented Sep 13, 2024

Great Switcheroo Phase Two. This PR refactors the redirects files and inverts www.zooniverse.org's default location / behavior from routing to PFE/blob storage to routing to FEM at the fe-root app. The PFE redirect files now contain the desired exceptions to this new default behavior.

One giant exception is /projects, which uses a prefix matcher to point at PFE (with exception-exceptions made for /assets and /_next, which are always FEM). The list of "FEM project redirects" remains and is kept in its own file. This continues to be a list of explicitly FEM projects as the default remains PFE.

An important note is that the behavior of https://frontend.preview.zooniverse.org has been left entirely alone. This will still display PFE at the home page and can be used as before. If this behavior should also change, let me know.

frontend.preview behavior is the primary FEM testing domain, so it was updated to match production.

Merging this PR will deploy staging and allow for testing on static-staging.zooniverse.org via the staging proxy. Links may not work, but I'll be testing routes and ensuring that behavior works as intended. I also included an example "FEM project redirect" in the staging conf for testing purposes (/projects/brooke/i-fancy-cats). Deploying to production will be done in concert with FEM and PFE deploys next Tuesday and should probably be accompanied by a full CDN cache purge.

Copy link
Member

@lcjohnso lcjohnso left a comment

Choose a reason for hiding this comment

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

Looking good! Just a few minor questions.

nginx-fem-project-redirects.conf Outdated Show resolved Hide resolved
@@ -0,0 +1,75 @@
set $proxy_path "www.zooniverse.org";

# FEM project assets
Copy link
Member

Choose a reason for hiding this comment

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

Does the ordering of this directive, above and separate from the rest of the projects/ pieces (starting on L37), matter?

Copy link
Member Author

@zwolf zwolf Sep 13, 2024

Choose a reason for hiding this comment

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

Yeah, the ordering is important in that it's gotta be before the other projects matcher. And though I grouped it with the other asset matcher, it could be moved further down.

In this case, before matching with ~* ^/projects, I want to check for FEM assets (the assets and _next folders) and return them first as they're exceptions to the exceptions. I could switch the latter to a prefix matcher since they have a lower priority than regex matches, but this way seems more obvious: match ^/projects plus those folders explicitly and route to FEM, or match just ^/projects and point to PFE.

The FEM project redirects file itself is included first for the same reason.

# Example FEM project redirect
location ~* ^/projects/(?:[\w-]*?/)?brooke/i-fancy-cats/?(?:(classify|about)(?:/.+?)?)?/?$ {
resolver 1.1.1.1;
proxy_pass "https://fe-project.preview.zooniverse.org";
Copy link
Member

Choose a reason for hiding this comment

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

Just flagging one specific case to ask a syntax question: should direct assignment of proxy_pass URLs be written without double quotes (this one here uses quotes, as do a few other places in this PR). Acknowledging and guessing it probably doesn't matter, but wanted to check.

Copy link
Member Author

Choose a reason for hiding this comment

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

As far as I know it doesn't matter and we're just kind of inconsistent in this repo. You need quotes to avoid escaping certain characters but otherwise it works either way.

@zwolf
Copy link
Member Author

zwolf commented Sep 16, 2024

PR now includes newest FEM project redirect.

@zwolf zwolf merged commit 7c135c5 into master Sep 16, 2024
2 checks passed
@zwolf zwolf deleted the fem-switcharoo-phase-two branch September 16, 2024 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants