Skip to content

Migration lep#520

Open
kpsherva wants to merge 6 commits into
CERNDocumentServer:masterfrom
kpsherva:migration-lep
Open

Migration lep#520
kpsherva wants to merge 6 commits into
CERNDocumentServer:masterfrom
kpsherva:migration-lep

Conversation

@kpsherva
Copy link
Copy Markdown
Contributor

@kpsherva kpsherva commented May 4, 2026

it is best to review commit by commit
PR introduces some slight performance improvements
needs: CERNDocumentServer/cds-rdm#795

@kpsherva kpsherva requested a review from zubeydecivelek May 4, 2026 08:13
@kpsherva kpsherva moved this to In review 🔍 in Sprint Q2 2026 ☀️ May 4, 2026
Comment thread cds_migrator_kit/rdm/records/load/load.py
Comment thread cds_migrator_kit/rdm/records/load/load.py
Comment thread cds_migrator_kit/rdm/records/transform/transform.py
Comment thread cds_migrator_kit/rdm/records/transform/transform.py Outdated
Comment thread cds_migrator_kit/rdm/records/transform/xml_processing/rules/base.py
Comment thread cds_migrator_kit/rdm/records/transform/models/research.py Outdated
Comment thread cds_migrator_kit/rdm/records/transform/models/research_committee.py Outdated
Comment thread cds_migrator_kit/rdm/records/transform/models/research.py
Comment on lines +466 to +472
if subject.get("subject", "") in ["xx"]:
del subject

subjects(json_entry)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since we already check in the rule, you think it's needed? Or if we need should we also check select: other subjects to delete?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes we can move the whole clean up here. Let's move it after merging your PR, when we have all the values to drop?

"bookchapter",
"itcerntalk",
"slides",
"antarescerntalk" "slides",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"antarescerntalk",
"slides",

is this intended?

* chore(tests): fix testing cases, formatting


def test_migrate_comments_dry_run(temp_dir):
def test_migrate_comments_dry_run(temp_dir, test_app, db):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this needed?

}


# @model.over("_approvals", "^903__")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Will this be needed?
I see that we also removed role from the tests?

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