-
Notifications
You must be signed in to change notification settings - Fork 7
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
Files workflow #169
Conversation
kpsherva
commented
Jul 25, 2024
•
edited
Loading
edited
- closes Migration dedicated node cds-rdm#175
1b94903
to
4eba27b
Compare
There was a problem hiding this 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" |
There was a problem hiding this comment.
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
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 |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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