Conversation
daeb398 to
76e7b03
Compare
| ) | ||
|
|
||
| DEFAULT_PAGINATION_SIZE: int = 30 # TODO: Use dynamic pagination size (see #1424) | ||
| DEFAULT_STAKER_PAGINATION_SIZE_LIGHT_NODE: int = int(os.environ.get( |
There was a problem hiding this comment.
This pattern worries me. First, the pagination size feels like a more high-level setting; whoever calls StakingEscrowAgent should worry about setting it. Second, the low-level class's behavior is influenced by an envvar, which may lead to some hard to diagnose problems. The only other place where we do that is ContractEventsThrottler.
There was a problem hiding this comment.
The caller of StakingEscrowAgent can still proactively specify a pagination_size see get_all_active_stakers() which is the only place a pagination size is needed/used. If the user doesn't specify a pagination_size (i.e. None), only then is the default used.
We were already using a default value for light nodes in prior code because they act differently than regular nodes. However, the number was fixed and could not be modified meaning that if we were wrong on the value it couldn't be modified in the field without changing the code. To me, this was the best compromise between allowing the user to specify a pagination size, if not use a default, if the default failed then it could be modified proactively.
The goal here is not to have the env var ever set, but rather have it act as a fallback just in case, for some reason, our beliefs about what the pagination size should be are incorrect.
Regarding "hard to diagnose" problems, for extra help, we log when the default pagination size is used and its value. see L324.
There was a problem hiding this comment.
I don't mind the default, or that there's an envvar for the default, it just seems that this specific spot is too deep in the call stack to depend on external environment. I admit I can't think of a good solution at the moment.
Regarding "hard to diagnose" problems, for extra help, we log when the default pagination size is used and its value.
It helps, but it's still a surprising side effect.
…sulas endpoing and log any exceptions.
…d query parameters.
…eated and used for tests. exec_work_order tests now actually use /reencrypt ursulas endpoints.
…out URL query parameters and the usage.
…sher_verifying_key.
…r /exec_work_order. Documentation cleanup: use version 6.0.0 in output, 401 error status code.
… about url encoding in docs. Add unit test for Base64BytesRepresentation field. Cleanup imports.
Allow query parameters to be passed in the URL
Porter implementation of `/exec_work_order` (associated with TMapConKFrags for `/reencrypt` endpoint)
piotr-roslaniec
left a comment
There was a problem hiding this comment.
I reviewed some of the PRs I previously missed out on. LGTM 🦝
cygnusv
left a comment
There was a problem hiding this comment.
Checked individual PRs, and ran porter via CLI and docker. This is an awesome work! 💪
|
@derekpierre - What do you make of the CI failure? https://app.circleci.com/pipelines/github/nucypher/nucypher/8212/workflows/73bacae3-e320-4803-a63e-14ab170a8210/jobs/139336 |
@KPrasch, it was introduced by the latest rebase which excluded a needed code change - should be fixed now. Thanks for checking! |
KPrasch
left a comment
There was a problem hiding this comment.
Stellar effort here @derekpierre 👏🏻
("Porter" is also a Canadian airline based out of Toronto - https://blog.flyporter.com/)
CHAMPION: @derekpierre
Goal
Simplifies and abstracts the complexities surrounding the nucypher protocol. It aims to negate the need for running the python client by applications to interact with the network - think web and mobile applications. It would leverage
rust-umbraland associated JS bindings to achieve cross-platform functionality via the browser, without needing to rewrite the entire python client.web app <--> "js-nucypher" <--> porter <--> networkRelated to https://github.com/nucypher/productdev/issues/7.
MVP Milestone: https://github.com/nucypher/nucypher/milestone/18
High-level areas of focus for MVP:
Scope the functionality that needs to be provided eg. learning loop, sampling, kfrag distribution, payment,...? ( Scope Porter functionality needed by Alice #2666 , Scope Porter functionality needed by Bob #2667 )
Porter Basic Learner (Basic Porter "Learner" #2668)
REST App/CLI Plumbing (REST App/CLI built around Porter "Learner" #2702)
REST API Implementation (based on work done in Scope Porter functionality needed by Alice #2666 and Scope Porter functionality needed by Bob #2667 )
Containerization and Deployment/Running of Porter, eg. with docker (Containerization/Deployment of Porter #2669 )
Documentation (Documentation of Porter #2670 )
Association with
js-nucypher- connective tissue betweenrust-umbral/webapps andporter(Issue(s) TBD)PR Ramp Order
/get_treasure_map,/publish_treasure_map, and/get_ursulas(unaffected by TMapConKFrags work) #2717/exec_work_order(associated with TMapConKFrags for/reencryptendpoint) #2729