You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@libcloud.apache.org by anthony shaw <an...@gmail.com> on 2016/04/04 12:27:40 UTC
[dev] [DISCUSS] - Replacing httplib with requests
As per the request of a number of our users, I've been working on a PR
to replace the base HTTP connection classes with an implementation
using the requests library.
For the following reasons
1. requests is the defacto standard - it would be in the standard
library but agreed against to allow it to develop faster
https://github.com/kennethreitz/requests/issues/2424
2. it works with python 2.6->3.5
3. Our SSL implementation is bad for Windows users (yes, they do
exist!) they have to download a copy of the certificate authority
database from some random website, I've seen security people turn to
stone whilst reading this
4. Developers can use requests_mock for deeper integration testing
5. less code to maintain
The PR is #728 https://github.com/apache/libcloud/pull/728
This started out as what I thought would be a simple change to update
LibcloudHTTPSConnection and LibcloudHTTPConnection, after refactoring
then I realised the classes were now identical and moved them into 1
instance. There is no longer a need for conn_classes tuple in the
Connection class, which is now a conn_class for a single class
instance.
This change ended up being less about the implementation of the base
classes, which was quite simple really, but about updating tests.
Aside from the Yak shaving I noticed a few things:
1) the tests were coupled to there being a conn_classes tuple, of the
6300+ tests, I broken 4300 in the first few commits
2) some of the test classes had a method called "respond" and due to
Pythons multiple inheritance algorithm this covered the underlying
response classes' implementation.
3) Our chunking support mocks for storage tests were designed to
always pass, I've implemented a string iterator for the tests, but
this still doesn't seem like a fair test
So, after updating all of the test classes due to a combination of
test fragility, changing the API (and the test mocks making
assumptions about the existence and behaviour of internal fields), I
now don't have the confidence that a 100% test pass means 0
regression.
I think the best approach is not to merge this PR into trunk, but into
a seperate branch and allow further testing as a seperate package,
e.g. apache-libcloud-requests
Please share your thoughts, concerns and ideas
Anthony
Re: [dev] [DISCUSS] - Replacing httplib with requests
Posted by Tomaz Muraus <to...@apache.org>.
First of all, nice work!
Overall, I'm fine with those changes as long as it meets this criteria:
1. It reduces number of lines and amount of code we need to support
2. It works with all the Python versions we support
3. "External" API stays the same for developers
4. Our libcloud.security "wrapper" stays the same
5. Docs need to be updated (notably on how SSL related settings are
manipulated)
Also, to add some context on why we didn't requests in the first place.
When the Libcloud project was started, requests project didn't exist yet so
we didn't have much choice but write our own HTTP layer. In addition to
that, package manager at that time (pip) was also very basic.
Both of that has changed over the past couple of years and pip has also
improved a lot in the last 1-2 years so I'm fine with using and adding some
"reasonable" amount of core dependencies (hey, no leftpad! :D).
On Mon, Apr 4, 2016 at 12:27 PM, anthony shaw <an...@gmail.com>
wrote:
> As per the request of a number of our users, I've been working on a PR
> to replace the base HTTP connection classes with an implementation
> using the requests library.
>
> For the following reasons
>
> 1. requests is the defacto standard - it would be in the standard
> library but agreed against to allow it to develop faster
> https://github.com/kennethreitz/requests/issues/2424
> 2. it works with python 2.6->3.5
> 3. Our SSL implementation is bad for Windows users (yes, they do
> exist!) they have to download a copy of the certificate authority
> database from some random website, I've seen security people turn to
> stone whilst reading this
> 4. Developers can use requests_mock for deeper integration testing
> 5. less code to maintain
>
> The PR is #728 https://github.com/apache/libcloud/pull/728
>
> This started out as what I thought would be a simple change to update
> LibcloudHTTPSConnection and LibcloudHTTPConnection, after refactoring
> then I realised the classes were now identical and moved them into 1
> instance. There is no longer a need for conn_classes tuple in the
> Connection class, which is now a conn_class for a single class
> instance.
>
> This change ended up being less about the implementation of the base
> classes, which was quite simple really, but about updating tests.
>
> Aside from the Yak shaving I noticed a few things:
> 1) the tests were coupled to there being a conn_classes tuple, of the
> 6300+ tests, I broken 4300 in the first few commits
> 2) some of the test classes had a method called "respond" and due to
> Pythons multiple inheritance algorithm this covered the underlying
> response classes' implementation.
> 3) Our chunking support mocks for storage tests were designed to
> always pass, I've implemented a string iterator for the tests, but
> this still doesn't seem like a fair test
>
> So, after updating all of the test classes due to a combination of
> test fragility, changing the API (and the test mocks making
> assumptions about the existence and behaviour of internal fields), I
> now don't have the confidence that a 100% test pass means 0
> regression.
>
> I think the best approach is not to merge this PR into trunk, but into
> a seperate branch and allow further testing as a seperate package,
> e.g. apache-libcloud-requests
>
> Please share your thoughts, concerns and ideas
>
> Anthony
>
Re: [dev] [DISCUSS] - Replacing httplib with requests
Posted by anthony shaw <an...@gmail.com>.
Please read and share my post on the changes and how to try them out.
http://libcloud.apache.org/blog/2016/04/06/requests-support.html
Anthony
On Wed, Apr 6, 2016 at 11:10 AM, anthony shaw <an...@gmail.com> wrote:
> I've dropped a package for anyone wanting to test this out on my
> personal apache space.
>
> They are signed with my PGP key as usual.
>
> pip install -e http://people.apache.org/~anthonyshaw/libcloud/1.0.0-rc2-requests/apache-libcloud-1.0.0-rc2-requests.zip@feature#egg=apache-libcloud
>
> The hashes are on the space
> http://people.apache.org/~anthonyshaw/libcloud/1.0.0-rc2-requests/
>
> Anthony
>
> On Mon, Apr 4, 2016 at 11:22 PM, Markos Gogoulos <mg...@mist.io> wrote:
>> I don't think I will have the time during this week to test this, would
>> just like to say a big 'Bravo' for this work!
>>
>>
>>
>> On Mon, Apr 4, 2016 at 1:27 PM, anthony shaw <an...@gmail.com>
>> wrote:
>>
>>> As per the request of a number of our users, I've been working on a PR
>>> to replace the base HTTP connection classes with an implementation
>>> using the requests library.
>>>
>>> For the following reasons
>>>
>>> 1. requests is the defacto standard - it would be in the standard
>>> library but agreed against to allow it to develop faster
>>> https://github.com/kennethreitz/requests/issues/2424
>>> 2. it works with python 2.6->3.5
>>> 3. Our SSL implementation is bad for Windows users (yes, they do
>>> exist!) they have to download a copy of the certificate authority
>>> database from some random website, I've seen security people turn to
>>> stone whilst reading this
>>> 4. Developers can use requests_mock for deeper integration testing
>>> 5. less code to maintain
>>>
>>> The PR is #728 https://github.com/apache/libcloud/pull/728
>>>
>>> This started out as what I thought would be a simple change to update
>>> LibcloudHTTPSConnection and LibcloudHTTPConnection, after refactoring
>>> then I realised the classes were now identical and moved them into 1
>>> instance. There is no longer a need for conn_classes tuple in the
>>> Connection class, which is now a conn_class for a single class
>>> instance.
>>>
>>> This change ended up being less about the implementation of the base
>>> classes, which was quite simple really, but about updating tests.
>>>
>>> Aside from the Yak shaving I noticed a few things:
>>> 1) the tests were coupled to there being a conn_classes tuple, of the
>>> 6300+ tests, I broken 4300 in the first few commits
>>> 2) some of the test classes had a method called "respond" and due to
>>> Pythons multiple inheritance algorithm this covered the underlying
>>> response classes' implementation.
>>> 3) Our chunking support mocks for storage tests were designed to
>>> always pass, I've implemented a string iterator for the tests, but
>>> this still doesn't seem like a fair test
>>>
>>> So, after updating all of the test classes due to a combination of
>>> test fragility, changing the API (and the test mocks making
>>> assumptions about the existence and behaviour of internal fields), I
>>> now don't have the confidence that a 100% test pass means 0
>>> regression.
>>>
>>> I think the best approach is not to merge this PR into trunk, but into
>>> a seperate branch and allow further testing as a seperate package,
>>> e.g. apache-libcloud-requests
>>>
>>> Please share your thoughts, concerns and ideas
>>>
>>> Anthony
>>>
Re: [dev] [DISCUSS] - Replacing httplib with requests
Posted by anthony shaw <an...@gmail.com>.
I've dropped a package for anyone wanting to test this out on my
personal apache space.
They are signed with my PGP key as usual.
pip install -e http://people.apache.org/~anthonyshaw/libcloud/1.0.0-rc2-requests/apache-libcloud-1.0.0-rc2-requests.zip@feature#egg=apache-libcloud
The hashes are on the space
http://people.apache.org/~anthonyshaw/libcloud/1.0.0-rc2-requests/
Anthony
On Mon, Apr 4, 2016 at 11:22 PM, Markos Gogoulos <mg...@mist.io> wrote:
> I don't think I will have the time during this week to test this, would
> just like to say a big 'Bravo' for this work!
>
>
>
> On Mon, Apr 4, 2016 at 1:27 PM, anthony shaw <an...@gmail.com>
> wrote:
>
>> As per the request of a number of our users, I've been working on a PR
>> to replace the base HTTP connection classes with an implementation
>> using the requests library.
>>
>> For the following reasons
>>
>> 1. requests is the defacto standard - it would be in the standard
>> library but agreed against to allow it to develop faster
>> https://github.com/kennethreitz/requests/issues/2424
>> 2. it works with python 2.6->3.5
>> 3. Our SSL implementation is bad for Windows users (yes, they do
>> exist!) they have to download a copy of the certificate authority
>> database from some random website, I've seen security people turn to
>> stone whilst reading this
>> 4. Developers can use requests_mock for deeper integration testing
>> 5. less code to maintain
>>
>> The PR is #728 https://github.com/apache/libcloud/pull/728
>>
>> This started out as what I thought would be a simple change to update
>> LibcloudHTTPSConnection and LibcloudHTTPConnection, after refactoring
>> then I realised the classes were now identical and moved them into 1
>> instance. There is no longer a need for conn_classes tuple in the
>> Connection class, which is now a conn_class for a single class
>> instance.
>>
>> This change ended up being less about the implementation of the base
>> classes, which was quite simple really, but about updating tests.
>>
>> Aside from the Yak shaving I noticed a few things:
>> 1) the tests were coupled to there being a conn_classes tuple, of the
>> 6300+ tests, I broken 4300 in the first few commits
>> 2) some of the test classes had a method called "respond" and due to
>> Pythons multiple inheritance algorithm this covered the underlying
>> response classes' implementation.
>> 3) Our chunking support mocks for storage tests were designed to
>> always pass, I've implemented a string iterator for the tests, but
>> this still doesn't seem like a fair test
>>
>> So, after updating all of the test classes due to a combination of
>> test fragility, changing the API (and the test mocks making
>> assumptions about the existence and behaviour of internal fields), I
>> now don't have the confidence that a 100% test pass means 0
>> regression.
>>
>> I think the best approach is not to merge this PR into trunk, but into
>> a seperate branch and allow further testing as a seperate package,
>> e.g. apache-libcloud-requests
>>
>> Please share your thoughts, concerns and ideas
>>
>> Anthony
>>
Re: [dev] [DISCUSS] - Replacing httplib with requests
Posted by Markos Gogoulos <mg...@mist.io>.
I don't think I will have the time during this week to test this, would
just like to say a big 'Bravo' for this work!
On Mon, Apr 4, 2016 at 1:27 PM, anthony shaw <an...@gmail.com>
wrote:
> As per the request of a number of our users, I've been working on a PR
> to replace the base HTTP connection classes with an implementation
> using the requests library.
>
> For the following reasons
>
> 1. requests is the defacto standard - it would be in the standard
> library but agreed against to allow it to develop faster
> https://github.com/kennethreitz/requests/issues/2424
> 2. it works with python 2.6->3.5
> 3. Our SSL implementation is bad for Windows users (yes, they do
> exist!) they have to download a copy of the certificate authority
> database from some random website, I've seen security people turn to
> stone whilst reading this
> 4. Developers can use requests_mock for deeper integration testing
> 5. less code to maintain
>
> The PR is #728 https://github.com/apache/libcloud/pull/728
>
> This started out as what I thought would be a simple change to update
> LibcloudHTTPSConnection and LibcloudHTTPConnection, after refactoring
> then I realised the classes were now identical and moved them into 1
> instance. There is no longer a need for conn_classes tuple in the
> Connection class, which is now a conn_class for a single class
> instance.
>
> This change ended up being less about the implementation of the base
> classes, which was quite simple really, but about updating tests.
>
> Aside from the Yak shaving I noticed a few things:
> 1) the tests were coupled to there being a conn_classes tuple, of the
> 6300+ tests, I broken 4300 in the first few commits
> 2) some of the test classes had a method called "respond" and due to
> Pythons multiple inheritance algorithm this covered the underlying
> response classes' implementation.
> 3) Our chunking support mocks for storage tests were designed to
> always pass, I've implemented a string iterator for the tests, but
> this still doesn't seem like a fair test
>
> So, after updating all of the test classes due to a combination of
> test fragility, changing the API (and the test mocks making
> assumptions about the existence and behaviour of internal fields), I
> now don't have the confidence that a 100% test pass means 0
> regression.
>
> I think the best approach is not to merge this PR into trunk, but into
> a seperate branch and allow further testing as a seperate package,
> e.g. apache-libcloud-requests
>
> Please share your thoughts, concerns and ideas
>
> Anthony
>