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.

Sign up for GitHub

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

Jump to bottom

Migrations won't work if the storage backend is Azure or S3 #2843

Closed
dralley opened this issue Oct 18, 2022 · 3 comments · Fixed by #2853
Closed

Migrations won't work if the storage backend is Azure or S3 #2843

dralley opened this issue Oct 18, 2022 · 3 comments · Fixed by #2853
Assignees
@pavelpicka
Labels
Issue prio-list

Comments

@dralley
Copy link
Contributor

dralley commented Oct 18, 2022

Version

3.18

Describe the bug

This line uses the os module, which will fail when the storage backend in use is not the local filesystem.

https://github.com/pulp/pulp_rpm/blob/main/pulp_rpm/app/migrations/0044_noartifact_modules.py#L18

Our tests are passing only because there is no data in the system prior to the migration: #2829
@dralley
Copy link
Contributor Author

dralley commented Oct 19, 2022

It looks like there are also issues with https://github.com/pulp/pulp_rpm/blob/main/pulp_rpm/app/migrations/0003_DATA_incorrect_json.py

Which ought to be fixed at the same time

@dralley
Copy link
Contributor Author

dralley commented Oct 19, 2022 •
edited

Alternatively, we could just say "these backends aren't supported until version X" if it's difficult

@pavelpicka pavelpicka self-assigned this Oct 24, 2022
@dralley dralley changed the title Migration 0044 won't work if the storage backend is Azure or S3 Migrations won't work if the storage backend is Azure or S3 Nov 1, 2022
pavelpicka added a commit to pavelpicka/pulp_rpm that referenced this issue Nov 3, 2022
@pavelpicka
Use default storage instead of os module
0f9f932 
avoid using ``os.path...`` for support all storage backedns

closes: pulp#2843
@pavelpicka pavelpicka mentioned this issue Nov 3, 2022
Merged
pavelpicka added a commit to pavelpicka/pulp_rpm that referenced this issue Nov 3, 2022
@pavelpicka
Use default storage instead of os module
fa2dcff 
avoid using ``os.path...`` for support all storage backedns

closes: pulp#2843
dralley pushed a commit that referenced this issue Nov 3, 2022
@pavelpicka @dralley
Use default storage instead of os module
3475bc3 
avoid using ``os.path...`` for support all storage backedns

closes: #2843
patchback bot pushed a commit that referenced this issue Nov 3, 2022
@pavelpicka @patchback
Use default storage instead of os module
aa0be7d 
avoid using ``os.path...`` for support all storage backedns

closes: #2843
(cherry picked from commit 3475bc3)
@patchback patchback bot mentioned this issue Nov 3, 2022
Merged
dralley pushed a commit that referenced this issue Nov 7, 2022
@pavelpicka @dralley
Use default storage instead of os module
71a6fb6 
avoid using ``os.path...`` for support all storage backedns

closes: #2843
(cherry picked from commit 3475bc3)
@dralley
Copy link
Contributor Author

dralley commented Nov 7, 2022

@markgoddard I'm doing a release now (3.18.8) with the fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@pavelpicka pavelpicka

Labels
Issue prio-list
Projects
RH Pulp Kanban board
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Use default storage instead of os module pavelpicka/pulp_rpm
2 participants
@dralley @pavelpicka