-
Notifications
You must be signed in to change notification settings - Fork 181
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
gergo/web 2158 previews module multi region #3492
Conversation
📸 Preview service has generated an image. |
@iainsproat i'd like to get some help fixing the preview service test job. It does not work the way you set it up any more. We cannot inject the DB all the way down cause of multi region project selection I've tested locally, and both mainDb and regional previews are generated. |
📸 Preview service has generated an image. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i guess so?
} | ||
|
||
start() | ||
.then() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the .then()/.catch() bits are redundant
|
||
const isDevEnv = process.env.NODE_ENV === 'development' | ||
|
||
export const getDbClients = async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is already better than it was, but in the future we can probably even extract a lot of this DB querying/"repository" logic that is common between these BG services into shared as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think in future, preview service should be given a short-lived API token and call the API on the server instead of duplicating the objects stuff.
But that's a discussion & refactor for the future, not one to hold up this PR.
…gergo/web-2158-previews-module-multi-region
📸 Preview service has generated an image. |
📸 Preview service has generated an image. |
Description & motivation
Changes:
To-do before merge:
Screenshots:
Validation of changes:
Checklist:
References