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

Sync storage with make up flow #22805

Merged
merged 2 commits into from
Nov 12, 2024
Merged

Sync storage with make up flow #22805

merged 2 commits into from
Nov 12, 2024

Conversation

KevinMind
Copy link
Contributor

@KevinMind KevinMind commented Oct 30, 2024

Relates to: mozilla/addons#15066
Child of: #22781

Description

Sync the storage director with make initialize command. 1) when you dump/load a db backup it will store/load the storage directory as well (correctly) 2) when seeding the db it will sync the storage directory as well 3) when syncing the storage directory it will clear the directory first to ensure pristine backups 4) remove the storage volume and just mount the repo and map the files correctly to static or storage directories.

Context

Currently data_seed is semi broken as it does not sync the storage directory when creating a snapshot.

Testing

run a clean init

NOTE: this will remove your existing DB so create a backup if you want to restore after

make up INIT_CLEAN=True

Expect the storage directory to be cleared and populated with the data from addons created during the seed.

run a backup and restore.

  1. have a running environment
  2. upload an addon
  3. create a backup make data_dump
  4. upload another addon
  5. run a restore make data_load
  6. expect the files associated with your 1st addon to be restored but not the 2nd.

Checklist

  • Add #ISSUENUM at the top of your PR to an existing open issue in the mozilla/addons repository.
  • Successfully verified the change locally.
  • The change is covered by automated tests, or otherwise indicated why doing so is unnecessary/impossible.
  • Add before and after screenshots (Only for changes that impact the UI).
  • Add or update relevant docs reflecting the changes made.

@KevinMind KevinMind force-pushed the sync-storage branch 4 times, most recently from ce0f6b8 to 6d2fdaf Compare October 30, 2024 20:25
@KevinMind KevinMind requested review from a team, eviljeff and diox and removed request for a team and eviljeff October 31, 2024 08:26
@diox
Copy link
Member

diox commented Nov 8, 2024

Saw that service "web" refers to undefined volume storage: invalid compose project error in CI ?

@diox
Copy link
Member

diox commented Nov 8, 2024

run a restore make data_restore

You mean make data_load ARGS="--name <backupname>" right ? data_restore doesn't exist.

@diox
Copy link
Member

diox commented Nov 8, 2024

Interesting issue I ran into:

  • I was logged in
  • I ran the dump
  • I re-initialized my db
  • I logged in again (creating a new user, since this is a fresh db)
  • I ran the restore
  • I refreshed the /developers/ page, and I was redirected to the login flow, except I got that error:
SessionInterrupted at /api/v5/accounts/login/start/

The request's session was deleted before the request completed. The user may have logged out in a concurrent request, for example.

Digging into it, this appears to be because we're using cached db session engine and Django is using "new" cache with "old" data, so that doesn't quite work properly. Indeed, doing a from django.core.cache import cache; cache.clear() fixed the issue.

Since this PR is about syncing up storage with the db, maybe it makes sense to address this here as well - That error can manifest in other ways, and it's a very similar underlying problem of cache not being consistent with the db backup we just loaded. I assume we should just clear the cache after a data_load to fix this.

@diox
Copy link
Member

diox commented Nov 8, 2024

expect the files associated with your 1st addon to be restored but not the 2nd.

I can still see files from my second add-on in the storage directory (even though it doesn't exist in the db anymore)

@KevinMind
Copy link
Contributor Author

@diox:

  • the first issue about invalid volume is a bug from rebasing, now fixed.
  • updated the instructions you are right it is data_load
  • added the clear cache method in data_load after we do the db restore. tried reproducing your issue and could not
  • added logic to clear the storage dir before syncing from the backup that should fix the issue of stale data post data load

@KevinMind KevinMind force-pushed the sync-storage branch 2 times, most recently from be4a889 to 574e14c Compare November 8, 2024 19:23
@KevinMind KevinMind merged commit b3d0a22 into master Nov 12, 2024
31 checks passed
@KevinMind KevinMind deleted the sync-storage branch November 12, 2024 15:32
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