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

Add CLI arg for preserving exchange records #355

Merged
merged 2 commits into from Feb 26, 2020

Conversation

dbluhm
Copy link
Member

@dbluhm dbluhm commented Jan 31, 2020

The toolbox taps into ACA-Py's credential exchange records to get a record of credentials issued; deleting these records once the exchange completes makes it a little harder to accurately show a list of issued credentials. The added command line argument gives the option to preserve those records after the exchange has completed.

Signed-off-by: Daniel Bluhm <daniel.bluhm@sovrin.org>
@codecov-io
Copy link

Codecov Report

Merging #355 into master will decrease coverage by <.01%.
The diff coverage is 85.71%.

@@            Coverage Diff             @@
##           master     #355      +/-   ##
==========================================
- Coverage   89.23%   89.23%   -0.01%     
==========================================
  Files         236      236              
  Lines       11179    11184       +5     
==========================================
+ Hits         9976     9980       +4     
- Misses       1203     1204       +1

@swcurran
Copy link
Member

@andrewwhitehead @nrempel - I leave it to you to decide on this one. We had agreed that agent storage should NOT be used for protocol state objects except while in flight. If the controller wants to keep the state objects, that should be done by the controller.

That said, we used to support this, and it was useful for some demo use cases.

If we do add this want to make it clear that this should probably not be used in any production use cases and developers of controllers should plan accordingly to have the controller manage such data.

Your thoughts?

@andrewwhitehead
Copy link
Member

--preserve-exchange-records would seem to apply to presentation exchange records as well, maybe it should?

@dbluhm
Copy link
Member Author

dbluhm commented Feb 3, 2020

--preserve-exchange-records would seem to apply to presentation exchange records as well, maybe it should?

That would make sense to me; this actually points out one thing that seemed inconsistent to me. At least at the point that I branched from master, presentation exchange records are not deleted in the built in handlers for present-proof. I can add that and then similarly gate it with this setting but I also didn't want to introduce deletion by default when it wasn't already present.

@swcurran
Copy link
Member

@dbluhm - are you going to add the preservation of presentations that was discussed above? I think there was confusion on that, which is why this was held up.

We'd definitely like that added before this feature is in a tagged release (e.g. 0.4.3).

@andrewwhitehead andrewwhitehead merged commit 2f6164f into hyperledger:master Feb 26, 2020
@dbluhm dbluhm deleted the preserve-exchange-records branch October 5, 2020 23:34
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