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

Added study distribution #144

Merged
merged 7 commits into from
Jan 7, 2025
Merged

Added study distribution #144

merged 7 commits into from
Jan 7, 2025

Conversation

dogversioning
Copy link
Contributor

This PR adds a mechanism for shipping prepared studies from github directly to sites, by adding a new lambda and an SNS queue with cross-account subscription permissions.

Copy link

github-actions bot commented Dec 17, 2024

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
863 843 98% 90% 🟢

New Files

File Coverage Status
src/dashboard/post_distribute/init.py 100% 🟢
src/dashboard/post_distribute/post_distribute.py 100% 🟢
src/dashboard/queue_distribute/init.py 100% 🟢
src/dashboard/queue_distribute/queue_distribute.py 100% 🟢
TOTAL 100% 🟢

Modified Files

File Coverage Status
src/dashboard/get_chart_data/get_chart_data.py 97% 🟢
src/dashboard/get_csv/get_csv.py 92% 🟢
src/dashboard/get_metadata/get_metadata.py 100% 🟢
src/dashboard/get_study_periods/get_study_periods.py 100% 🟢
src/site_upload/cache_api/cache_api.py 94% 🟢
src/site_upload/fetch_upload_url/fetch_upload_url.py 93% 🟢
src/site_upload/powerset_merge/powerset_merge.py 95% 🟢
src/site_upload/process_flat/process_flat.py 100% 🟢
src/site_upload/process_upload/process_upload.py 98% 🟢
src/site_upload/study_period/study_period.py 100% 🟢
TOTAL 97% 🟢

updated for commit: b20e82b by action🐍

src/post_distribute.Dockerfile Outdated Show resolved Hide resolved
src/post_distribute.Dockerfile Outdated Show resolved Hide resolved
template.yaml Outdated Show resolved Hide resolved
template.yaml Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
200,
),
("invalid_study", "https://github.com/smart-on-fhir/invalid_study", 500),
("non_cumulus_repo", "https://github.com/user/non_cumulus_repo", 500),
Copy link
Contributor

Choose a reason for hiding this comment

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

This line is trying to test that a ValueError is thrown when no smart-on-fhir string is detected, yeah? Can this test code distinguish from the case where the code does try to hit that URL but no responses mock has been set up for it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

probably with some work.

this would be easier if we could use something other than the generic error handler, but security has requested these endpoints be extremely vague in their error reporting until the dashboard is deployed inside AWS.

src/dashboard/post_distribute/post_distribute.py Outdated Show resolved Hide resolved
src/dashboard/post_distribute/post_distribute.py Outdated Show resolved Hide resolved
src/dashboard/post_distribute/post_distribute.py Outdated Show resolved Hide resolved
src/dashboard/post_distribute/post_distribute.py Outdated Show resolved Hide resolved
src/dashboard/post_distribute/post_distribute.py Outdated Show resolved Hide resolved
src/dashboard/post_distribute/post_distribute.py Outdated Show resolved Hide resolved
src/dashboard/post_distribute/post_distribute.py Outdated Show resolved Hide resolved
Copy link
Contributor Author

@dogversioning dogversioning left a comment

Choose a reason for hiding this comment

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

Addressed some PR comments.

Note that this also decouples 'i receieved a message' from 'i'm taking a long while to build a study', since API gateway automatically throws a timeout error at 29 seconds of runtime.

src/dashboard/post_distribute/post_distribute.py Outdated Show resolved Hide resolved
src/dashboard/post_distribute/post_distribute.py Outdated Show resolved Hide resolved
src/dashboard/post_distribute/post_distribute.py Outdated Show resolved Hide resolved
src/dashboard/post_distribute/post_distribute.py Outdated Show resolved Hide resolved
src/dashboard/queue_distribute/queue_distribute.py Outdated Show resolved Hide resolved
src/dashboard/queue_distribute/queue_distribute.py Outdated Show resolved Hide resolved
src/dashboard/post_distribute/post_distribute.py Outdated Show resolved Hide resolved
parsed_url = urllib.parse.urlparse(config["url"])
if (
not parsed_url.netloc == "github.com"
or not parsed_url.path.startswith("/smart-on-fhir")
Copy link
Contributor

Choose a reason for hiding this comment

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

Should add a trailing slash, else the smart-on-fhir-for-realz org can send URLs.

@dogversioning dogversioning merged commit 57b87c7 into main Jan 7, 2025
2 checks passed
@dogversioning dogversioning deleted the mg/sns-fanning branch January 7, 2025 14:09
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.

2 participants