Skip to content
This repository has been archived by the owner on Jul 13, 2023. It is now read-only.

further refine Handler validation #695

Closed
pjenvey opened this issue Oct 12, 2016 · 2 comments · Fixed by #870
Closed

further refine Handler validation #695

pjenvey opened this issue Oct 12, 2016 · 2 comments · Fixed by #870
Assignees
Labels

Comments

@pjenvey
Copy link
Member

pjenvey commented Oct 12, 2016

Improvements we can do here:

o how we pass validation results to handlers. @threaded_validate currently attaches valid_input to the handler obj yet continues calling the handler method w/ unvalidated kwargs (which we always ignore and are thus confusing/dangerous to even keep around).

We should probably pass **valid_input as the kwargs instead

o We can also pass thru richer objects (like a fully constructed WebPushNotification object for e.g. in web/message.py). Either as fields.Raw or in a @post_load (not clear to me yet what works better)?

@pjenvey pjenvey added this to the PUSHSVC-0: quality milestone Oct 12, 2016
@pjenvey pjenvey self-assigned this Oct 12, 2016
@jrconlin
Copy link
Member

I'd prefer if we used fields.Raw. passing back as part of @post_load feels too "magical" for my tastes. We declare the fields at the top of the class and should stick to that so we don't lose data.

@bbangert bbangert added p1 and removed p2 labels Nov 14, 2016
@pjenvey pjenvey added p3 and removed p1 labels Feb 9, 2017
@pjenvey pjenvey added in progress and removed ready labels Apr 3, 2017
pjenvey added a commit that referenced this issue Apr 4, 2017
- params -> router_data, kill unused vapid_info/body
- kill bogus/unused timing vars
- app_id isn't optional

issue #695
pjenvey added a commit that referenced this issue Apr 4, 2017
- try to isolate it to the deferred definition and avoid passing args
back and forth when not necessary
- also put all_channels call back in a thread

issue #695
pjenvey added a commit that referenced this issue Apr 4, 2017
- params -> router_data, kill unused vapid_info/body
- kill bogus/unused timing vars
- app_id isn't optional

issue #695
pjenvey added a commit that referenced this issue Apr 4, 2017
- try to isolate it to the deferred definition and avoid passing args
back and forth when not necessary
- also put all_channels call back in a thread

issue #695
pjenvey added a commit that referenced this issue Apr 4, 2017
- try to isolate it to the deferred definition and avoid passing args
back and forth when not necessary
- also put all_channels call back in a thread

issue #695
pjenvey added a commit that referenced this issue Apr 4, 2017
- try to isolate it to the deferred definition and avoid passing args
back and forth when not necessary
- also put all_channels call back in a thread

issue #695
pjenvey added a commit that referenced this issue Apr 4, 2017
- try to isolate it to the deferred definition and avoid passing args
back and forth when not necessary
- also put all_channels call back in a thread

issue #695
pjenvey added a commit that referenced this issue Apr 5, 2017
- try to isolate it to the deferred definition and avoid passing args
back and forth when not necessary
- also put all_channels call back in a thread

issue #695
pjenvey added a commit that referenced this issue Apr 6, 2017
- params -> router_data, kill unused vapid_info/body
- kill bogus/unused timing vars
- app_id isn't optional

issue #695
pjenvey added a commit that referenced this issue Apr 6, 2017
- try to isolate it to the deferred definition and avoid passing args
back and forth when not necessary
- also put all_channels call back in a thread

issue #695
pjenvey added a commit that referenced this issue Apr 13, 2017
- instead of valid_input call handler methods w/ the resulting dict as
args/kwargs (similarly to how cyclone does)
- bundle router info together in registration to ease passing it around
- kill the now unneeded MessageSchema args/kwargs shuffle and its
associated test_delete_topic_success2
- fix LogCheckSchema typo

closes #695
pjenvey added a commit that referenced this issue Apr 13, 2017
more accurately reflects what's happening now and cyclone's
path_args/kwargs are exactly the same as its calling args/kwargs
anyway

issue #695
pjenvey added a commit that referenced this issue Apr 13, 2017
- instead of valid_input call handler methods w/ the resulting dict as
args/kwargs (similarly to how cyclone does)
- bundle router info together in registration to ease passing it around
- kill the now unneeded MessageSchema args/kwargs shuffle and its
associated test_delete_topic_success2
- fix LogCheckSchema typo

closes #695
pjenvey added a commit that referenced this issue Apr 13, 2017
more accurately reflects what's happening now and cyclone's
path_args/kwargs are exactly the same as its calling args/kwargs
anyway

issue #695
@pjenvey pjenvey added p1 and removed p3 labels Apr 13, 2017
@pjenvey
Copy link
Member Author

pjenvey commented Apr 13, 2017

While shaking this out it seemed like @post_load is maybe more appropriate (webpush.py already uses it). fields.Raw would require the @post_loadish code to live inside of the @validates_schema methods most of the time (and we already have some larger ones).

Almost tempted to document what the schema produces to the handler in schema class docstring to make it very clear

pjenvey added a commit that referenced this issue Apr 13, 2017
- instead of valid_input call handler methods w/ the resulting dict as
args/kwargs (similarly to how cyclone does)
- bundle router info together in registration to ease passing it around
- kill the now unneeded MessageSchema args/kwargs shuffle and its
associated test_delete_topic_success2
- fix LogCheckSchema typo

closes #695
pjenvey added a commit that referenced this issue Apr 13, 2017
more accurately reflects what's happening now and cyclone's
path_args/kwargs are exactly the same as its calling args/kwargs
anyway

issue #695
pjenvey added a commit that referenced this issue Apr 13, 2017
- instead of valid_input call handler methods w/ the resulting dict as
args/kwargs (similarly to how cyclone does)
- bundle router info together in registration to ease passing it around
- kill the now unneeded MessageSchema args/kwargs shuffle and its
associated test_delete_topic_success2
- fix LogCheckSchema typo

closes #695
pjenvey added a commit that referenced this issue Apr 14, 2017
- instead of valid_input call handler methods w/ the resulting dict as
args/kwargs (similarly to how cyclone does)
- bundle router info together in registration to ease passing it around
- kill the now unneeded MessageSchema args/kwargs shuffle and its
associated test_delete_topic_success2
- fix LogCheckSchema typo

closes #695
pjenvey added a commit that referenced this issue Apr 14, 2017
- instead of valid_input call handler methods w/ the resulting dict as
args/kwargs (similarly to how cyclone does)
- bundle router info together in registration to ease passing it around
- kill the now unneeded MessageSchema args/kwargs shuffle and its
associated test_delete_topic_success2
- fix LogCheckSchema typo

closes #695
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants