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

Add more tests for ngi_data and fix some bugs #29

Merged
merged 7 commits into from
Jan 8, 2024

Conversation

aanil
Copy link
Member

@aanil aanil commented Jan 8, 2024

No description provided.

@aanil aanil added the no validation validation not needed label Jan 8, 2024
@aanil aanil requested a review from alneberg January 8, 2024 10:02
@aanil aanil linked an issue Jan 8, 2024 that may be closed by this pull request
@@ -7,4 +7,4 @@ DAILY_READ_REPORTS_LOCATION='tests/test_reports_location'
DAILY_READ_FETCH_FROM_NGIS = true
DAILY_READ_FETCH_FROM_SNPSEQ = true
DAILY_READ_FETCH_FROM_UGC = true
#DAILY_READ_LOG_LOCATION='tests/log_location'
DAILY_READ_USERS_LIST_LOCATION=
Copy link
Member

Choose a reason for hiding this comment

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

Ok, replaced a comment with a likely typo? Shouldn't there be something on the right side of the equal sign?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah no, its not a typo. The field can be empty and should be empty if you want DailyRead to look at all users. If its not, the reports will only be generated for users in the list.

Copy link
Member

@alneberg alneberg left a comment

Choose a reason for hiding this comment

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

Looks good, thank you!

@aanil aanil merged commit b86606e into NationalGenomicsInfrastructure:main Jan 8, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no validation validation not needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use a single source for status priority
2 participants