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: using a default mediator #1490

Merged
merged 2 commits into from
Nov 14, 2021
Merged

Conversation

dbluhm
Copy link
Member

@dbluhm dbluhm commented Nov 13, 2021

This fixes an error in the use of a default mediator in the connections and out of band protocols. The mediation ID was being saved as None instead of the retrieved default mediator value.

This fixes an error in the use of a default mediator in the connections
protocol. The mediation ID was being saved as None instead of the
retrieved default mediator value.

Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
@dbluhm
Copy link
Member Author

dbluhm commented Nov 13, 2021

Unit tests are failing due to JSONLD context resolution errors 🤔

@dbluhm
Copy link
Member Author

dbluhm commented Nov 13, 2021

I would really like to get this fix into 0.7.2 if possible. @swcurran

@swcurran
Copy link
Member

Will do. @ianco , @shaangill025 or @TimoGlastra want to review?

@ianco
Copy link
Member

ianco commented Nov 14, 2021

The tests all ran on my local I'll try to re-run. (Probably failed due to solar flares)

Scratch that able to reproduce locally.

@ianco
Copy link
Member

ianco commented Nov 14, 2021

The tests all ran on my local I'll try to re-run. (Probably failed due to solar flares)

Scratch that able to reproduce locally.

Hmmm not consistent sometimes it passes, weird

@codecov-commenter
Copy link

Codecov Report

Merging #1490 (a3d8a15) into main (e1e95de) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1490   +/-   ##
=======================================
  Coverage   95.71%   95.71%           
=======================================
  Files         489      489           
  Lines       30264    30264           
=======================================
  Hits        28967    28967           
  Misses       1297     1297           

@swcurran swcurran merged commit 15d309c into hyperledger:main Nov 14, 2021
@swcurran
Copy link
Member

Any more idea on what is happening? I got a fail on merging this...

@swcurran
Copy link
Member

Looked at the error and the JSON-LD for the example is not loading. I tried the document in the JSON-LD Playground and it does indeed give an error.

  • Error Message: pyld.jsonld.JsonLdError: ('Dereferencing a URL did not result in a valid JSON-LD object. Possible causes are an inaccessible URL perhaps due to a same-origin policy (ensure the server uses CORS if you are using client-side JavaScript), too many redirects, a non-JSON response, or more than one HTTP Link Header was provided for a remote context.',) Type: jsonld.InvalidUrl Code: loading remote context failed Details: {'url': 'https://fhircat.org/fhir-r5/rdf-r5/contexts/patient.context.jsonld', 'cause': JsonLdError('Could not retrieve a JSON-LD document from the URL.',)} self = <urllib3.connection.HTTPSConnection object at 0x7f45dda01f28>
  • Failing URL: https://fhircat.org/fhir-r5/rdf-r5/contexts/patient.context.jsonld
  • JSON-LD Playground Error: jsonld.SyntaxError: Invalid JSON-LD syntax; invalid scoped context.

Not much help...from the me or from the error messages.

@swcurran
Copy link
Member

@shaangill025 -- looks like this is a consistent problem. I'm guessing something changed in the context. Should we remove this test from the suite for now? Can you please take a look this morning? I'd like this resolved for 0.7.2.

Thanks

@ianco
Copy link
Member

ianco commented Nov 15, 2021

Any more idea on what is happening? I got a fail on merging this...

This error was happening sporadically for me, I suspect some external dependency ...

@ianco
Copy link
Member

ianco commented Nov 15, 2021

Seems like "connection refused" on urls at https://fhircat.org/:

E               ConnectionRefusedError: [Errno 111] Connection refused

../.pyenv/versions/3.6.13/lib/python3.6/site-packages/urllib3/util/connection.py:86: ConnectionRefusedError

During handling of the above exception, another exception occurred:

self = <urllib3.connectionpool.HTTPSConnectionPool object at 0x7fabcda862b0>, method = 'GET', url = '/fhir-r5/rdf-r5/contexts/sampleddata.context.jsonld', body = None
headers = {'User-Agent': 'python-requests/2.25.1', 'Accept-Encoding': 'gzip, deflate', 'Accept': 'application/ld+json;profile=ht...application/ld+json, application/json;q=0.5, text/html;q=0.8, application/xhtml+xml;q=0.8', 'Connection': 'keep-alive'}
retries = Retry(total=0, connect=None, read=False, redirect=None, status=None), redirect = False, assert_same_host = False
timeout = Timeout(connect=None, read=None, total=None), pool_timeout = None, release_conn = False, chunked = False, body_pos = None
response_kw = {'decode_content': False, 'preload_content': False}
parsed_url = Url(scheme=None, auth=None, host=None, port=None, path='/fhir-r5/rdf-r5/contexts/sampleddata.context.jsonld', query=None, fragment=None)
destination_scheme = None, conn = None, release_this_conn = True, http_tunnel_required = False, err = None, clean_exit = False

The url's work fine for me manually and there are tests that randomly fail (it's not always the same test)

Not sure if we can add a retry? In this case it's getting pulled in through a dependency from http://hl7.org/fhir/Patient I think, so not sure we have direct control of it ...

@ianco
Copy link
Member

ianco commented Nov 15, 2021

Error is getting thrown from within the pyld package:

E           pyld.jsonld.JsonLdError: ('Could not retrieve a JSON-LD document from the URL.',)
E           Type: jsonld.LoadDocumentError
E           Code: loading document failed
E           Cause: HTTPSConnectionPool(host='fhircat.org', port=443): Max retries exceeded with url: /fhir-r5/rdf-r5/contexts/sampleddata.context.jsonld (Caused by NewConnectionError('<urllib3.connection.HTTPSConnection object at 0x7fabcda86860>: Failed to establish a new connection: [Errno 111] Connection refused',))  File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/site-packages/pyld/documentloader/requests.py", line 63, in loader
E               response = requests.get(url, headers=headers, **kwargs)
E             File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/site-packages/requests/api.py", line 76, in get
E               return request('get', url, params=params, **kwargs)
E             File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/site-packages/requests/api.py", line 61, in request
E               return session.request(method=method, url=url, **kwargs)
E             File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/site-packages/requests/sessions.py", line 542, in request
E               resp = self.send(prep, **send_kwargs)
E             File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/site-packages/requests/sessions.py", line 655, in send
E               r = adapter.send(request, **kwargs)
E             File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/site-packages/requests/adapters.py", line 516, in send
E               raise ConnectionError(e, request=request)

../.pyenv/versions/3.6.13/lib/python3.6/site-packages/pyld/documentloader/requests.py:103: JsonLdError

@dbluhm dbluhm deleted the fix/default-mediator branch September 17, 2022 14:35
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.

None yet

4 participants