Skip to content

Conversation

@PatStLouis
Copy link
Contributor

This is to address failing integration tests for revocation registry rotation:
https://github.com/openwallet-foundation/acapy/actions/runs/21226131058/job/61073454321?pr=4026

Some race conditions were causing the agent to try and set an active registry before it was stored locally.

@PatStLouis PatStLouis mentioned this pull request Jan 22, 2026
Signed-off-by: Patrick St-Louis <[email protected]>
Signed-off-by: Patrick St-Louis <[email protected]>
@swcurran
Copy link
Contributor

@ff137 -- don't know if you have time to look at this, but I know you have done a lot of work on revocation registry creation handling and errors. Your feedback would be appreciated.

@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
50.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

Copy link
Member

@ff137 ff137 left a comment

Choose a reason for hiding this comment

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

LGTM!

create_and_register_revocation_registry_definition returns a RevRegDefResult | string, and if it was a string then it means it's an error message. The current logic:

        if new_reg:
            new_rev_reg_def_id = new_reg.rev_reg_def_id

would try to access .rev_reg_def_id on a string... which would raise an error. And that's my bad. I previously modified that and missed this.

So the change seems good and necessary to avoid that runtime error.

(A type checker like mypy would have caught this. Definitely worth it to try add mypy to this project, although it would require a lot of mini typing changes)

@swcurran swcurran merged commit a5eb092 into openwallet-foundation:main Jan 22, 2026
11 of 12 checks passed
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.

3 participants