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

Support absolute uri in request_json #127

Merged
merged 9 commits into from
Nov 17, 2023
Merged

Support absolute uri in request_json #127

merged 9 commits into from
Nov 17, 2023

Conversation

devmotion
Copy link
Member

According to the specs (https://www.hl7.org/fhir/R4/datatypes.html#uri), uris can be relative and absolute. For instance, the links to the current and the next page in the paginated response to http://hapi.fhir.org/baseR4/Patient?given=Jason&family=Argonaut&_count=2&_pretty=true are URLs that already contain the base URL.

However, currently request_json does not support such paths but always assumes that the path string is relative to the base URL. With this PR also absolute paths are supported.


This PR is based on #125.

Base automatically changed from dw/compat_stdlib to master November 8, 2023 00:46
@DilumAluthge DilumAluthge self-requested a review November 8, 2023 00:57
Copy link

codecov bot commented Nov 8, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (aef5379) 100.00% compared to head (2c4f881) 100.00%.

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #127   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            8         8           
  Lines          178       187    +9     
=========================================
+ Hits           178       187    +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@DilumAluthge DilumAluthge left a comment

Choose a reason for hiding this comment

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

Might be good to add a few unit tests to make sure that we detect absolute vs relative paths correctly?

@devmotion devmotion mentioned this pull request Nov 8, 2023
src/requests.jl Outdated Show resolved Hide resolved
@devmotion
Copy link
Member Author

@DilumAluthge I added tests but #127 (comment) is not resolved yet - I'm fine with your suggestion as well, I'd just like to confirm that you would still prefer it 🙂

src/requests.jl Outdated Show resolved Hide resolved
src/requests.jl Outdated Show resolved Hide resolved
Co-authored-by: Dilum Aluthge <dilum@aluthge.com>
@devmotion devmotion merged commit be2ce2c into master Nov 17, 2023
15 checks passed
@devmotion devmotion deleted the dw/absolute_path branch November 17, 2023 15:27
@devmotion devmotion mentioned this pull request Nov 17, 2023
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