Adjust layout of multi-tenant groups bucket #562
Conversation
tsibley
force-pushed
the
trs/groups-storage-layout
branch
from
7a125cf
to
7753716
Compare
|
Ah, CI is failing because it's looking for test fixtures in a public group (blab) that aren't found because I haven't done the initial copy to the new layout (see deploy steps above). It should be safe to do that ahead of time (by design!), so I'll do that and CI should pass. |
|
Ah, actually, if I do that nothing will "break", but every migrated group's datasets and narratives will appear duplicated in the listings. I'll address that first with a separate PR to ignore those, in preparation for this PR. |
tsibley
force-pushed
the
trs/groups-storage-layout
branch
from
7753716
to
cb2c21f
Compare
…/scripts module Two generic functions that I wrote with reusability in mind, and now I have a need to re-use them.
The bucket now stores datasets and narratives separately under a fixed prefix within each group's prefix so the user-provided names/paths of the datasets and narratives can't conflict with control objects we store at the root of the group's prefix. Besides making security easier to reason about here with the user-provided names, this will also, in time, allow us to eliminate a few special-cased exclusions (like excluding group-overview.md from a list of *.md narratives). As part of deploying this, we'll need to perform a one-time manual layout switch behind the scenes using scripts/migrate-groups-layout. Though migrate-groups-layout is intended to be run only twice: 1. Immediately before deploy of the layout change. 2. Immediately after deploy of the layout change, with --delete-after-copy. …it is designed to be safe to run repeatedly. However, as it is a single-use program, it's not designed to live past those two primary invocations and we should delete it after it's served its purpose.
I'd been doing this manually while reviewing the previous sync logs, as before the layout change the sync just migrated everything even though some objects would be no longer accessible. Nicer to have it built-in as part of the rest of the manual steps checklist.
tsibley
force-pushed
the
trs/groups-storage-layout
branch
from
cb2c21f
to
c42ee78
Compare
There was a problem hiding this comment.
I can't speak to the technical details of the changes here, but I love the idea of splitting out datasets from narratives and providing a first-class support for these types of data in the code. Even though I don't know this code base well, I can tell that these changes make the code more readable in that we can look for a Narrative class instead of scanning for *.md to figure out if an action is applied to a narrative or not.
The migration is complete and this program is no longer needed. It worked great during the migration period, though, so good job to it! Related to <#562>.
|
🤦 This required a hotfix which I'm in the process of deploying: 90e5fcd More context in Slack: |
Description of proposed changes
The gist here is a behind-the-scenes layout change for the
nextstrain-groupsbucket to better separate objects within a group.Currently:
With this:
This has been in the back of my mind for a while, but was recently reinvigorated/remotivated by discussion of issues with the current layout when describing #547.
See commit messages for full details.
Related issue(s)
Related to #478.
Testing
migrate-groupscript withtest-privategroup (after "reverting" its migration)migrate-groups-layoutscript against a private test bucket using objects copied froms3://nextstrain-groups/test-private/.Deploy
to perform in order
./scripts/migrate-groups-layout --wet-runto do the initial copy../scripts/migrate-groups-layout --wet-run --delete-after-copyto pick up any intervening changes and do the final move../scripts/migrate-groups-layoutfile in a new commit.