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

EVA-3635 - Fixes from Testing #53

Merged
merged 8 commits into from
Aug 30, 2024

Conversation

tcezard
Copy link
Member

@tcezard tcezard commented Aug 21, 2024

Ensure all relative path are made into full path
Raise if metadata spreadsheet is not provided
Remove requirement for VCF file type since it has been removed from the metadata Output logs for main nextflow steps to help debug
Add debug option to main CLI
Lower some docker validator log message to debug

@tcezard tcezard marked this pull request as draft August 21, 2024 13:32
@tcezard tcezard changed the title Fixes from Testing EVA-3635 - Fixes from Testing Aug 21, 2024
# Validation is mandatory so if submit is requested then VALIDATE must have run before or be requested as well
if SUBMIT in tasks:
if not sub_config.get(READY_FOR_SUBMISSION_TO_EVA, False):
return True
submission_id = sub_config.get(SUB_CLI_CONFIG_KEY_SUBMISSION_ID, None)
if submission_id:
try:
submission_status = SubmissionWSClient().get_submission_status(submission_id)
submission_status = SubmissionWSClient(username, password).get_submission_status(submission_id)
Copy link
Member Author

Choose a reason for hiding this comment

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

Passing the username and password is required because the auth is a global variable and subsequent call will not have the username/password if it is initially created without them.

Copy link
Contributor

Choose a reason for hiding this comment

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

I can't remember, did you check whether it worked without the global auth?

Copy link
Member Author

Choose a reason for hiding this comment

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

I did try briefly and it didn't work first time so didn't push it since it wasn't pat of this ticket

Raise if metadata spreadsheet is not provided
Remove requirement for VCF file type since it has been removed from the metadata
Output logs for main nextflow steps to help debug
Add debug option to main CLI
Lower some docker validator log message to debug
@tcezard tcezard force-pushed the EVA3635_fix_from_test branch from e6bb427 to bafae45 Compare August 22, 2024 13:37
@tcezard tcezard marked this pull request as ready for review August 23, 2024 11:32
Copy link
Contributor

@apriltuesday apriltuesday left a comment

Choose a reason for hiding this comment

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

Just tiny comments, overall looks good!

eva_sub_cli/nextflow/validation.nf Outdated Show resolved Hide resolved
# Validation is mandatory so if submit is requested then VALIDATE must have run before or be requested as well
if SUBMIT in tasks:
if not sub_config.get(READY_FOR_SUBMISSION_TO_EVA, False):
return True
submission_id = sub_config.get(SUB_CLI_CONFIG_KEY_SUBMISSION_ID, None)
if submission_id:
try:
submission_status = SubmissionWSClient().get_submission_status(submission_id)
submission_status = SubmissionWSClient(username, password).get_submission_status(submission_id)
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't remember, did you check whether it worked without the global auth?

eva_sub_cli/submit.py Outdated Show resolved Hide resolved
tcezard and others added 2 commits August 30, 2024 16:50
Co-authored-by: April Shen <april.tuesday@gmail.com>
@tcezard tcezard merged commit 3ed9462 into EBIvariation:main Aug 30, 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.

3 participants