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

Feature/remove obsolete code #507

Merged
merged 9 commits into from
May 25, 2016
Merged

Feature/remove obsolete code #507

merged 9 commits into from
May 25, 2016

Conversation

hixi
Copy link
Contributor

@hixi hixi commented May 24, 2016

Reviewed by:

While working on #502 I found that we have some code around, we're not using. I cleaned this up as far as I was able to tell, with one exception: removing the countries altogether (they live on the excerpts now), but they need to be available for migrations, which is a bigger task #506, so not included here.

@@ -15,7 +15,6 @@ class RESTApiClient:

def __init__(self, service_base):
self.service_base = service_base
self.client = requests.session()
Copy link
Member

@das-g das-g May 24, 2016

Choose a reason for hiding this comment

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

As long as we're not using this session, we can indeed delete this code. But maybe we should use a session? (By using the session's get and post instead of request.get and request.post in our own get and post methods below.) We don't need cookie persistence, but connection-pooling might be good.

And it seems that requests.Session already can do what we implemented ourselves with _data_dict() and _default_headers(): http://docs.python-requests.org/en/master/api/#requests.Session.headers

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I realized that, and AFAIK that line stems from an idea where I implemented it this way - but forgot about it again, and didn't remember while reviewing.

I created the issue #508 for this, since this doesn't belong in here.

Copy link
Member

Choose a reason for hiding this comment

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

👍

@hixi
Copy link
Contributor Author

hixi commented May 25, 2016

@das-g I made some changes, so I took the liberty of removing your check-mark above.

@hixi hixi merged commit 7e3c65a into develop May 25, 2016
@hixi hixi deleted the feature/remove_obsolete_code branch May 25, 2016 09:16
@hixi hixi removed the in progress label May 25, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants