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

fix: improve FHIR-root-URL parsing to cover more cases #373

Merged
merged 1 commit into from
Jan 15, 2025

Conversation

mikix
Copy link
Contributor

@mikix mikix commented Jan 15, 2025

Before, we were just using some regexes on the URL, but let's actually use a parser and separate out the pieces.

This is better in general, but also happens to let us handle cases like the user putting an export URL like ".../$export?_type=..." on the shell, having the shell zero-out the $export string because it thinks it is a variable, and then us seeing a URL like "/?_type=..." (which isn't really a valid URL, but we will insert missing $export strings later in the process).

As part of this, I refactored some instances of URL parsing into one single FhirUrl class, which we can build upon in the future as needed.

Checklist

  • Consider if documentation (like in docs/) needs to be updated
  • Consider if tests should be added

@mikix mikix force-pushed the mikix/url-parsing branch from d6b7a26 to 9d4455e Compare January 15, 2025 13:20
@mikix mikix force-pushed the mikix/url-parsing branch from 9d4455e to a039bac Compare January 15, 2025 14:29
Copy link

github-actions bot commented Jan 15, 2025

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
3640 3586 99% 98% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
cumulus_etl/fhir/init.py 100% 🟢
cumulus_etl/fhir/fhir_client.py 100% 🟢
cumulus_etl/fhir/fhir_utils.py 100% 🟢
cumulus_etl/loaders/fhir/bulk_export.py 100% 🟢
cumulus_etl/loaders/fhir/export_log.py 100% 🟢
TOTAL 100% 🟢

updated for commit: 3a5a0e5 by action🐍

Before, we were just using some regexes on the URL, but let's
actually use a parser and separate out the pieces.

This is better in general, but also happens to let us handle cases
like the user putting an export URL like ".../$export?_type=..." on
the shell, having the shell zero-out the $export string because it
thinks it is a variable, and then us seeing a URL like "/?_type=..."
(which isn't really a valid URL, but we will insert missing $export
strings later in the process).

As part of this, I refactored some instances of URL parsing into one
single FhirUrl class, which we can build upon in the future as needed.
@mikix mikix force-pushed the mikix/url-parsing branch from a039bac to 3a5a0e5 Compare January 15, 2025 14:59
@mikix mikix merged commit 2395bb4 into main Jan 15, 2025
3 checks passed
@mikix mikix deleted the mikix/url-parsing branch January 15, 2025 15:17
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