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

DB Schema migration tool #9

Draft
wants to merge 2 commits into
base: fix/tx-sharing
Choose a base branch
from

Conversation

jessegeens
Copy link

Depends on #8

@glpatcern
Copy link
Member

Shouldn't this tool go to a separate repository? After all, it's a standalone tool, not a "plugin". I'd push it into https://gitlab.cern.ch/cernbox/ops/cbox-scripts/ (not a public repo, but a priori nobody out there has ever used Reva with the DB).

@jessegeens jessegeens force-pushed the feat/db-schema-migrator branch 29 times, most recently from 9a7baaf to 8023e8c Compare January 9, 2025 14:10
@jessegeens jessegeens force-pushed the feat/db-schema-migrator branch 8 times, most recently from c78d0be to c738e9d Compare January 13, 2025 07:55
@jessegeens jessegeens force-pushed the feat/db-schema-migrator branch from c738e9d to 67e9d7b Compare January 13, 2025 12:29
@jessegeens
Copy link
Author

Shouldn't this tool go to a separate repository? After all, it's a standalone tool, not a "plugin". I'd push it into https://gitlab.cern.ch/cernbox/ops/cbox-scripts/ (not a public repo, but a priori nobody out there has ever used Reva with the DB).

Let's do this after the migration is complete :)

@jessegeens jessegeens changed the base branch from master to fix/tx-sharing January 13, 2025 12:31
@glpatcern
Copy link
Member

OK, the understanding here is that the migration tool depends on other internal packages in this repo, so it's easier to compile it here in place. After the migration, we will move the two files in a separate repo along with basic instructions on how to compile them.


func migrateShareStatuses(ctx context.Context, migrator Migrator) {
// Check how many shares are to be migrated
count, err := getCount(migrator, "oc_share")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
count, err := getCount(migrator, "oc_share")
count, err := getCount(migrator, "oc_share_status")

},
User: s.recipient,
Hidden: s.state == -1, // Hidden if REJECTED
Synced: true, // for now, we always sync? or not? TODO
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say yes.. Had the same question in the other PR..

// ShareTypeFederatedCloudShare = 6
// ShareTypeSpaceMembership = 7
if s.ShareType == 0 || s.ShareType == 1 {
return &ShareOrLink{
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not asking to change, but wouldn't it there be a way to just return "interface" and then use reflection to check the type of object when doing the "create if"?

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.

3 participants