-
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
Use default wallet type askar for alice/faber demo and bdd tests #1761
Conversation
Signed-off-by: Ian Costanzo <[email protected]>
Two of the integration tests are failing, revocation, The step that is failing is:
Looking into it ... |
The Some kind of database locking issue ... @andrewwhitehead ? |
That sounds pretty important :-). |
I think what's happening is this chunk of code here: ... creates and uses a transaction for the duration of the code block, however this line here:
... doesn't propagate the transaction, so then when the ledger update is done here: ... and then within the ledger code it tries to lookup the DID in the wallet: ... it's not using the transaction that is already open, so I think that makes it block. |
I notice there are some updates in progress to askar so not sure if that will address this issue or we need to fix this issue in aca-py code? I'll leave this for now pending feedback from @andrewwhitehead |
I don't think the latest changes would impact this. There shouldn't generally be an issue unless it's running out of available pool connections. |
It's definitely hitting a database lock. When I move this line There are database rows getting updated during the transaction and locking the table, and the DID lookup is happening on a different db connection (I assume) and not able to fetch the row from the (locked) table. |
The code works with postgres but not with sqlite Publishing revocation updates works when you auto-publish when the credential is revoked, but not when you later publish revocations (batched) |
Codecov Report
@@ Coverage Diff @@
## main #1761 +/- ##
=======================================
Coverage 93.72% 93.72%
=======================================
Files 536 536
Lines 34015 34015
=======================================
Hits 31881 31881
Misses 2134 2134 |
Any ideas on getting past this? Would be really good to have askar as the default going forward for ACA-Py. |
@andrewwhitehead mentioned he could look at this once the other revocation PR is merged. I think we were talking about adding a new state for the individual credential revocation, but now that we have a method to fix the problem I don't think it's as big of an issue of the ledger gets out of sync ... |
Actually I think there's a few more references to update: in docker-compose.yml, alice-local.sh, faber-local.sh, and runners/support/utils.py (check_requires method) |
This comment was marked as outdated.
This comment was marked as outdated.
@shaangill025 I think that these errors are due to the agent shutting down prior to all of the messages being sent (a connection is still in progress), and they don't seem to affect the result of the test:
It looks like the TAA-related tests should be filtered with |
Signed-off-by: shaangill025 <[email protected]>
…t-python into feat/default-askar-additions
@ianco Based upon Andrew's comment, I opened the following PR [ |
Set wallet-type to Askar across Demo
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.
Looks like an integration test is still failing.
Hmm runs successfully on my local, I'll take a look ... |
Signed-off-by: Ian Costanzo <[email protected]>
Adjusted tests, as one ( Also incldued 2 new tests in GHA that were failing locally with askar (but worked with indy):
... works, but:
fails. |
|
This is the most troubling issue for me related to the 0.7.4 release. @andrewwhitehead - any progress on this? AFAIK -- the schema, creddef and revreg gets created, the credential issued, the proof request with revocation sent and then the Prover (Bob) fails the proof generation process, but only with Askar. At that level, how can we get differences between Indy and Askar? Is that an indication of some low-level problem? |
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.
Integration tests now working - ready to merge. Good stuff!~
Signed-off-by: Ian Costanzo [email protected]