-
Notifications
You must be signed in to change notification settings - Fork 7
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
Depend on Tahoe-LAFS 1.17.1 #282
Conversation
Codecov Report
@@ Coverage Diff @@
## main #282 +/- ##
==========================================
- Coverage 91.76% 91.76% -0.01%
==========================================
Files 51 51
Lines 4677 4689 +12
Branches 643 645 +2
==========================================
+ Hits 4292 4303 +11
- Misses 336 338 +2
+ Partials 49 48 -1
Continue to review full report at Codecov.
|
src/_zkapauthorizer/_plugin.py
Outdated
@@ -140,7 +140,8 @@ def get_storage_server( | |||
registry=registry, | |||
) | |||
storage_server = ZKAPAuthorizerStorageServer( | |||
anonymous_storage_server, | |||
# unwrap the Foolscap layer, we'll do it ourselves. | |||
anonymous_storage_server._server, |
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.
Based on running the PrivateStorageio system tests, this is incorrect, as anonymous_storage_server
is already a StorageServer
, not a FoolscapStorageServer
.
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.
Thanks for this catch. I'm surprised this is what Tahoe is doing now ... but since it is easier to deal with than what I expected, I guess I won't complain. 🙂
This suggests an integration test would be useful.
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.
This looks good. There are a couple of minor things to change in the tests (mostly around naming to align with upstream), but is good to merge after that. (I assume that most of the coverage annotation things are due to codecov not working properly)
Stacked on #278
Fixes #280