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

Files workflow #169

Merged
merged 2 commits into from
Jul 26, 2024
Merged

Files workflow #169

merged 2 commits into from
Jul 26, 2024

Conversation

kpsherva
Copy link
Contributor

@kpsherva kpsherva commented Jul 25, 2024

@kpsherva kpsherva force-pushed the files-workflow branch 4 times, most recently from 1b94903 to 4eba27b Compare July 25, 2024 14:30
Copy link
Contributor

@jrcastro2 jrcastro2 left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

Just a couple of minor comments

@@ -355,3 +355,4 @@ def _(x): # needed to avoid start time failure with lazy strings
base_path = os.path.dirname(os.path.realpath(__file__))
logs_dir = os.path.join(base_path, "tmp/logs/")
CDS_MIGRATOR_KIT_LOGS_PATH = logs_dir
INVENIO_CDS_MIGRATOR_KIT_STREAM_CONFIG = "cds_migrator_kit/rdm/migration/streams.yaml"
Copy link
Contributor

Choose a reason for hiding this comment

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

minor: I would remove invenio

Suggested change
INVENIO_CDS_MIGRATOR_KIT_STREAM_CONFIG = "cds_migrator_kit/rdm/migration/streams.yaml"
CDS_MIGRATOR_KIT_STREAM_CONFIG = "cds_migrator_kit/rdm/migration/streams.yaml"

@@ -7,11 +7,19 @@

"""CDS-RDM migration load module."""

import os
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: This can be removed

parent_dest_path = os.path.dirname(destination_path)
if not os.path.exists(parent_dest_path):
os.makedirs(parent_dest_path)
shutil.copy(full_path, destination_path)
Copy link
Contributor

Choose a reason for hiding this comment

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

discussed to use the eosclient and permission based tokens that should replace the copy command here and the need for kinit


```shell
ssh cds-wn-31 # inveniomigrator tool installed here
kinit cdsrdmeosdev
Copy link
Contributor

Choose a reason for hiding this comment

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

see comment about using eosclient instead

@kpsherva kpsherva merged commit ba955cf into CERNDocumentServer:master Jul 26, 2024
1 check passed
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.

Migration dedicated node
3 participants