-
Notifications
You must be signed in to change notification settings - Fork 516
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
CWU - W3C Issue Credential #1061
Conversation
Signed-off-by: Timo Glastra <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
Signed-off-by: Karim Stekelenburg <[email protected]>
Signed-off-by: Karim Stekelenburg <[email protected]>
Signed-off-by: Karim Stekelenburg <[email protected]>
Signed-off-by: Karim Stekelenburg <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
Signed-off-by: Karim Stekelenburg <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
I see the cred ex record webhook event doesn't include the detail record. But the API does return it. @sklump is that on purpose? |
Hmmm we can't emit webhooks for stateless records anymore. Seems like this PR that was merged yesterday changed that: https://github.com/hyperledger/aries-cloudagent-python/pull/1063/files#diff-fe738d195b46caab22aaa9e802de272264dc1f986e589aad2a2bd0965f98ac14R382 OK -- need to wait for #1123 to be resolved |
Signed-off-by: Timo Glastra <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1061 +/- ##
==========================================
- Coverage 99.20% 98.87% -0.34%
==========================================
Files 383 427 +44
Lines 22139 24118 +1979
==========================================
+ Hits 21964 23847 +1883
- Misses 175 271 +96 |
Signed-off-by: Timo Glastra <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
c189260
to
c6a1626
Compare
All tests are passing. Ready for review! |
w00t!!!! Congrats! @ianco @andrewwhitehead -- check it out! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to add something like this to config.py?
extras_require={
"indy": parse_requirements("requirements.indy.txt"),
"bbs": parse_requirements("requirements.bbs.txt"),
"uvloop": {"uvloop": "^=0.14.0"},
},
Or what are the options for installing the new ursa lib?
Signed-off-by: Timo Glastra <[email protected]>
Ah I was looking how that worked with the extra requirements but couldn't find anything so I assumed it would be automatically detected based on the filename. Thanks! This explains why I couldn't get it to work in AATH :) |
I tried with the above update in aca-py and then installed as There are a couple of errors in AATH I think related to the |
Once this PR is merged we can flip the uncommented line with the commented line (at the link you posted @ianco) and it should work |
Signed-off-by: Timo Glastra <[email protected]>
@ianco Seems like we're dealing with some race conditions on the integration tests. It sporadically fails at different tests, but always with the same error. Have you seen this before?
|
@TimoGlastra I haven't seen that error before. I'm able to run the integration tests for this PR locally with no issues. |
BTW I'm also getting these two failures in AATH (with this PR plus fixing the line identified earlier):
(Something to do with |
Signed-off-by: Timo Glastra <[email protected]>
@ianco it should work now by running the open PR in AATH (with a pull) |
The detail records have webhooks of their own. |
But any cred exchange can only result in one credential issued (on one format): otherwise the state machine turns into an n-dimensional quantity. |
From the issue cred v2 RFC:
I interpret this in the way that you could issue multiple credentials as long as they are semantically the same. seems useful IMO |
Most noticeable breaking changes are in the wallet. Because of the addition of new key types and did methods it had to be extended.