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

WIP: alternate mode for repeating response handlers #256

Closed
wants to merge 1 commit into from

Conversation

cdent
Copy link
Owner

@cdent cdent commented Jul 9, 2018

This is an alternate version of pull #254, with the difference
in gabbi/suitemaker.py where response_handlers is only extended
if the response_* key has not been seen before. This means that
if the YAMLDiskLoadingJSONHandler is turned on it does all the
work of the JSONHandler for respone handling, because it is only
present once.

At the same time, the existing handing for content handlers means
that custom handlers which accept a content-type are used first (and
only) for the content type it accepts.

The issues with pytest and files having the same name remain
here. I tried to make some change to pytest test loading to
make the global cache use more distinguishing keys, but it was
distracting from progressing on the other work, so I'll deal with
that separately.

This is an alternate version of pull #254, with the difference
in gabbi/suitemaker.py where response_handlers is only extended
if the response_* key has not been seen before. This means that
if the YAMLDiskLoadingJSONHandler is turned on it does all the
work of the JSONHandler for respone handling, because it is only
present once.

At the same time, the existing handing for content handlers means
that custom handlers which accept a content-type are used first (and
only) for the content type it accepts.

The issues with pytest and files having the same name remain
here. I tried to make some change to pytest test loading to
make the global cache use more distinguishing keys, but it was
distracting from progressing on the other work, so I'll deal with
that separately.
@cdent
Copy link
Owner Author

cdent commented Jul 9, 2018

We don't necessarily want to merge this (in part because it uses a private member), but it provides a demo of a potentially cleaner way to deal with duplicated response handlers.

@trevormccasland what do you think?

trevormccasland pushed a commit to trevormccasland/gabbi that referenced this pull request Jul 10, 2018
this patch aims to integrate the changes made in[1] revert changes
related to the class attribute 'add_response_handler' in[2] and
add unit tests to cover the suitemaker logic from[1].

[1]cdent#256
[2]cdent@241684e
@cdent
Copy link
Owner Author

cdent commented Jul 16, 2018

#253 fixed this, so closing

@cdent cdent closed this Jul 16, 2018
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.

1 participant