feat(server): new projects are placed in a configurable region #3801
Conversation
…gergo/web-2429-new-projects-are-placed-in-a-configurable-region
| ...(args.input || {}), | ||
| ownerId: context.userId!, | ||
| ownerResourceAccessRules: context.resourceAccessRules | ||
| regionKey |
There was a problem hiding this comment.
what does regionKey do here? This should have a more explicit name for this object parameter.
There was a problem hiding this comment.
It somehow seems like a duplication that we have to pass it to getDb and here. It seems like this could be the source of error (that we mistakenly pass in a regionKey here but different projectDb to the factory)
There was a problem hiding this comment.
It is a property of the project. Given, that all functions are only implementing a domain operation, no DB clients are exposed to the business logic functions, so explicitly entering the region key is the appropriate solution on the right level.
| const regionKey = await getValidDefaultProjectRegionKey() | ||
| const projectDb = await getDb({ regionKey }) | ||
|
|
||
| const createNewProject = createNewProjectFactory({ |
There was a problem hiding this comment.
Why use this over createStreamReturnRecord? What's the purpose of createStreamReturnRecord and where should it be used, and can we add comments to document that? i.e. why was it being used here previously, and why is it no longer required?
I think this is the bit of context I was missing.
There was a problem hiding this comment.
createStreamReturnRecord is the old implementation, if you check how its written, its not compatible for creating multi region projects. That's why i had to write a new service for creating projects. The old one should not be used any more, but I did not have a need to replace it so far.
There was a problem hiding this comment.
Does createStreamReturnRecord need to be replaced everywhere it's currently referenced? e.g. will we continue to have onboarding streams created in the incorrect region if it's not replaced?
There was a problem hiding this comment.
Probably we'll replace it there too, but that is not a big deal right now, so i did not want this PR to balloon.
|
📸 Preview service has generated an image. |
iainsproat
changed the title
gjedlicska
deleted the
gergo/web-2429-new-projects-are-placed-in-a-configurable-region
branch
Description & motivation
Changes:
To-do before merge:
Screenshots:
Validation of changes:
Checklist:
References