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

chore(app-backend): Use v9 #7435

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

chore(app-backend): Use v9 #7435

wants to merge 5 commits into from

Conversation

VIKTORVAV99
Copy link
Member

@VIKTORVAV99 VIKTORVAV99 commented Nov 21, 2024

Issue

Closes: AVO-641
Closes: AVO-649

Description

Needs: https://github.com/electricitymaps/electricitymaps/pull/5975

Switches the paths to v9 and enables auth for the state path.

It also switches the tokens and env variables to the correct one for v9.

Double check

  • I have run pnpx prettier@2 --write . and poetry run format in the top level directory to format my changes.

@VIKTORVAV99
Copy link
Member Author

All hands on deck to review this one to ensure I haven't missed some usage of the old token and that it works as expected with the auth. If you need the new token for local evaluation let me know :)

Copy link
Collaborator

@tonypls tonypls left a comment

Choose a reason for hiding this comment

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

LGTM, approving but let's make the backend PR too and try merge tomorrow

@@ -36,7 +36,7 @@ build:
FROM +prepare
RUN pnpm run create-generated-files
RUN pnpm version minor
RUN --secret SENTRY_AUTH_TOKEN --secret VITE_PUBLIC_ELECTRICITYMAP_PUBLIC_TOKEN=ELECTRICITYMAP_PUBLIC_TOKEN pnpm run build
RUN --secret SENTRY_AUTH_TOKEN --secret VITE_PUBLIC_ELECTRICITYMAP_PUBLIC_TOKEN_V9=ELECTRICITYMAPS_APP_PUBLIC_TOKEN_V9 pnpm run build
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we need a backend PR to add this to .drone-earthly.yml

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we need a backend PR to add this to .drone-earthly.yml

Don't we get this from the secret manager directly?

And where is this file, I can't seem to find it in contrib? 👀

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's in the mono-repo. It's how earthly will get the secrets when it's building the bundle to deploy to prod

Copy link
Member Author

Choose a reason for hiding this comment

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

It's in the mono-repo. It's how earthly will get the secrets when it's building the bundle to deploy to prod

Really? 👀

I haven't been adding parser tokens there either as far as I remember and they still work. I'll double check though, but the backend deployment was working fine without it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Was that not for when we deployed it to instance 1 and ran it ourself rather than hosted it on Cloudflare?

Copy link
Collaborator

Choose a reason for hiding this comment

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

You could be right!

@VIKTORVAV99
Copy link
Member Author

LGTM, approving but let's make the backend PR too and try merge tomorrow

The first backend PR is already merged, that is why this is working ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants