Thanks,

it's also worth noting that the new connection design leverages the
requests session objects, which reuse TCP sessions between requests.
http://docs.python-requests.org/en/master/user/advanced/#session-objects

If you're making a lot of calls, e.g. polling or paging node data,
there will be a significant performance improvement on 1.5.0<

The HTTP mocking functionality is hugely improved. Requests has an
adapter interface, so you can consume a driver, connection class
without having to mock out ANYTHING except the underlying HTTP calls.
This is available as a context manager in requests_mock, which I use
for testing a lot of other API projects (it rocks).

+ conn = Connection(host='mock.com', port=80, secure=False)
+ conn.connect()
+
+ with requests_mock.Mocker() as m:
+     m.register_uri('GET', 'http://mock.com/raw_data', text=TEST_DATA,
+          headers={'test': 'value'})
+     response = conn.request('/raw_data')
+ data = response.text
+ self.assertEqual(data, TEST_DATA)

Ant

On Tue, Jan 10, 2017 at 8:28 PM, Allard Hoeve <[email protected]> wrote:
> Hey,
>
> I reviewed a little (but important) part of the PR.
>
> I'm planning to run the entire branch against an OpenStack provider,
> Digital Ocean and AWS EC2 soon, when I have the time.
>
> Best,
>
> Allard
>
> On Tue, Jan 10, 2017 at 9:46 AM anthony shaw <[email protected]>
> wrote:
>
>> I decided the best thing was to use this :
>>
>> https://github.com/apache/libcloud/pull/970
>>
>> To verify the changes. It's going to be a complete driver, both
>> compute and storage using a real API. No Mocking, No funny-business.
>> Uses authentication, checks custom headers, file uploads and downloads
>> etc. We can use this in our release process.
>>
>> I've already found 3 bugs using it, including one nasty one. The
>> RawResponse class actually exposed the underlying httplib.HTTPResponse
>> class as a class property. I'm just writing a proxy class for it now.
>>
>> On Tue, Jan 10, 2017 at 7:41 PM, anthony shaw <[email protected]>
>> wrote:
>> > The regenerated provider tables? I had a merge conflict (well, many
>> > merge conflicts) on the way and re-running the table generator is the
>> > safest way to resolve those ones.
>> >
>> > They need running again, since the AWS regions are a dictionary, the
>> > ordering is random.
>> >
>> > On Tue, Jan 10, 2017 at 7:37 PM, Allard Hoeve <[email protected]>
>> wrote:
>> >> Hey Anthony,
>> >>
>> >> Wow, that is a lot of work :) Thanks for that.
>> >>
>> >> I started reviewing the branch, but it's 3k+, 3k- and I immediately ran
>> >> into the problem of "reviewing a patch that's too big". For example
>> >> <
>> https://github.com/apache/libcloud/pull/923/files#diff-f9b7105b8fba7a7f1a0ddec6ef14c8feR413
>> >:
>> >> why is that added in? Seems unrelated, but you must've done that for a
>> >> reason.
>> >>
>> >> So before the review devolves into a bunch of nitpicks, how do you
>> suppose
>> >> we'd review most efficiently?
>> >>
>> >> Maybe you can go through the PR and comment yourself on why certain
>> things
>> >> are as they are?
>> >>
>> >> Best,
>> >>
>> >> Allard
>> >>
>> >>
>> >> On Mon, Jan 9, 2017 at 5:40 AM anthony shaw <[email protected]>
>> >> wrote:
>> >>
>> >>> This pull-request is FINALLY finished! It's taken me the best part of
>> >>> a year to complete it.
>> >>>
>> >>> https://github.com/apache/libcloud/pull/923
>> >>>
>> >>> This last round of changes I had to completely rewrite the Storage API
>> >>> base classes. I've been testing using my GCP account and uploading and
>> >>> downloading files and it all looks good now. I've gone back and tested
>> >>> a bunch of random other drivers like GoDaddy and Dimension Data for
>> >>> which I have accounts for.
>> >>>
>> >>> I'm going to merge it and raise another PR this week with an
>> >>> integration suite module. This will have a driver and a FLASK web app
>> >>> with a tox definition to test the libcloud library from end to end.
>> >>> The way the httplib_ssl module is mocked out in our unit tests leaves
>> >>> a lot of room for mistakes.
>> >>>
>> >>> Things that visually don't make much sense but I don't have an account
>> to
>> >>> test
>> >>> - The code for Azure Blob leases looked fragile. This really needs
>> >>> testing properly
>> >>> - Multipart uploads should work for storage APIs, S3 has a custom API
>> >>> that is now disabled
>> >>>
>> http://docs.aws.amazon.com/AmazonS3/latest/dev/UsingRESTAPImpUpload.html
>> >>> - The Aliyun OSS driver had some really visible bugs in it, I doubt
>> >>> the existing driver works. We need integration testers for it.
>> >>>
>> >>>
>> >>>
>> >>> On Fri, Jan 6, 2017 at 8:05 PM, Samia, Michel <[email protected]>
>> wrote:
>> >>> > <html><bodyFor multipart upload you can use
>> >>> https://pypi.python.org/pypi/requests-toolbelt
>> >>> >
>> >>> > -----Original Message-----
>> >>> > From: anthony shaw [mailto:[email protected]]
>> >>> > Sent: Friday, January 6, 2017 6:07 AM
>> >>> > To: dev <[email protected]>
>> >>> > Subject: Re: [dev] [DISCUSS] Using requests instead of httplib
>> >>> >
>> >>> > Good news is I figured out a way of implementing the upload
>> >>> functionality using the requests package.
>> >>> >
>> >>> >
>> >>>
>> https://github.com/apache/libcloud/pull/923/commits/5e04dbce554830eca3f9812272076a2fbdbe7cdc
>> >>> >
>> >>> > I've tested it against the Google Cloud Storage account, downloaded
>> and
>> >>> uploaded a file using both the direct file_path option and the option
>> >>> passing a context manager (IOStream or ByteStream).
>> >>> >
>> >>> > The bad news is :
>> >>> > - The S3 multipart upload I've removed. I don't have time right now
>> to
>> >>> implement this feature from scratch
>> >>> > - The unit tests are all coupled to the private methods, the call
>> back
>> >>> system and a bunch of other bad coupling practices, so they are broken
>> BUT
>> >>> it does actually work
>> >>> >
>> >>> > It's nearly there.
>> >>> >
>> >>> > Ant
>> >>> >
>> >>> > On Fri, Jan 6, 2017 at 7:28 AM, anthony shaw <
>> [email protected]>
>> >>> wrote:
>> >>> >> it's more of an existential question :-)
>> >>> >>
>> >>> >> The _upload_object method inside the libcloud.storage.base submodule
>> >>> >> makes a 'raw' call to the LibcloudConnection, which will send the
>> top
>> >>> >> part of the HTTP request then some headers and leave the connection
>> >>> >> open (i.e. not read the response).
>> >>> >>
>> >>> >> Then, depending on the driver, the file and other things, it will
>> >>> >> callback one of the methods like _stream_data, which writes directly
>> >>> >> to the HTTP session using the `send()` method, which is only
>> available
>> >>> >> in httplib.
>> >>> >>
>> >>> >> httplib is a very low level library, requests is very high level.
>> You
>> >>> >> don't get access to the HTTP session directly in requests.
>> >>> >>
>> >>> >> That means that I would have to throw away the code we already have
>> >>> >> (which I am definitely in favour of in the long term since it is
>> >>> >> fragile) and replace it with requests' APIs for doing chunked
>> uploads
>> >>> >> using file streams.
>> >>> >>
>> >>> >> It would probably take me another day or two to finish that
>> >>> implementation.
>> >>> >>
>> >>> >> I always preached that you should change 1 thing at a thing, in
>> small
>> >>> >> amount, and keep testing. So far this has been more like pulling a
>> >>> >> thread on a sweater, I've touched every single file in the code base
>> >>> >> practically!
>> >>> >> The odds are, I will have missed something. So 2.0.0rc1 (if we do
>> call
>> >>> >> it that), despite my best intentions will introduce a new bug just
>> >>> >> based on the number of things I have changed.
>> >>> >>
>> >>> >> On Fri, Jan 6, 2017 at 1:11 AM, Tom Melendez <[email protected]>
>> wrote:
>> >>> >>> Hi Anthony,
>> >>> >>>
>> >>> >>> Nice job getting this going!
>> >>> >>>
>> >>> >>> Would you mind elaborating on this point?
>> >>> >>> "The raw connection still uses httplib. I decided it was too risky
>> to
>> >>> >>> swap that for requests' method of uploading files."
>> >>> >>>
>> >>> >>> Since you're going through the trouble, it would be ideal to go to
>> >>> >>> Requests completely.  What's blocking us on the upload code
>> >>> >>> (Admittedly, I haven't studied the upload code)?  Anything the
>> >>> community can do to help?
>> >>> >>>
>> >>> >>> Thanks,
>> >>> >>>
>> >>> >>> Tom
>> >>> >>>
>> >>> >>>
>> >>> >>> On Thu, Jan 5, 2017 at 3:43 AM, Chris DeRamus
>> >>> >>> <[email protected]>
>> >>> >>> wrote:
>> >>> >>>
>> >>> >>>> For what it's worth my company (DivvyCloud) has been using the
>> good
>> >>> >>>> work you've done now for almost six months. We had to make a few
>> >>> >>>> tweaks, but the core code contributed has worked flawlessly across
>> >>> >>>> AWS, Azure, OpenStack, Google, VMware and more. The only issue I
>> >>> >>>> believe that still stands which I've seen is an issue when
>> >>> >>>> LIBCLOUD_DEBUG is set to true. Logging doesn't appear to function
>> >>> >>>> properly, but that may have been addressed since your initial
>> >>> submission last year.
>> >>> >>>>
>> >>> >>>> Nice work on this and we sincerely appreciate the contribution.
>> >>> >>>>
>> >>> >>>> On Thu, Jan 5, 2017 at 12:21 AM, anthony shaw
>> >>> >>>> <[email protected]>
>> >>> >>>> wrote:
>> >>> >>>>
>> >>> >>>> > That package had a dumb error in it, I've since fixed it and
>> >>> >>>> > against a live API (GoDaddy). I've tested the following
>> scenarios
>> >>> >>>> >
>> >>> >>>> > - Applying a custom proxy via the environment variable
>> >>> >>>> > - Using libcloud.security to disable SSL verification
>> >>> >>>> > - Using libcloud.security to set a custom CA certificate
>> >>> >>>> > - Combining all of those scenarios
>> >>> >>>> > - Verification of custom headers applied by the driver using
>> >>> >>>> > Charles Proxy and inspecting the HTTP messages manually
>> >>> >>>> > - Decoding JSON messages - although this still uses the existing
>> >>> >>>> > methods, not the requests own json() decoder.
>> >>> >>>> >
>> >>> >>>> > IMO, this is ready to merge. I would like to test the raw
>> >>> >>>> > connections and file uploads if anyone has an account on one of
>> >>> those providers?
>> >>> >>>> >
>> >>> >>>> > On Thu, Jan 5, 2017 at 8:30 AM, Tomaz Muraus <[email protected]>
>> >>> wrote:
>> >>> >>>> > > Thanks for working on this again!
>> >>> >>>> > >
>> >>> >>>> > > Once we get a green light from people testing those changes, I
>> >>> >>>> > > propose
>> >>> >>>> to
>> >>> >>>> > > first roll out v2.0.0-rc1 and eventually after we are happy
>> with
>> >>> >>>> > > the stability call it v2.0.0.
>> >>> >>>> > >
>> >>> >>>> > > On Wed, Jan 4, 2017 at 6:20 AM, anthony shaw
>> >>> >>>> > > <[email protected]
>> >>> >>>> >
>> >>> >>>> > > wrote:
>> >>> >>>> > >
>> >>> >>>> > >> Hi,
>> >>> >>>> > >>
>> >>> >>>> > >> I tried doing this a year ago but put it in the 'too hard'
>> >>> bucket.
>> >>> >>>> > >> I've opened a PR (again) replacing the use of httplib with
>> the
>> >>> >>>> > >> requests package.
>> >>> >>>> > >>
>> >>> >>>> > >> The consequences are:
>> >>> >>>> > >> - Connection.conn_classes is no longer a tuple, it is
>> >>> >>>> > >> Connection.conn_class. There is no separation between a HTTP
>> >>> >>>> > >> and HTTPS connection. I could have just hacked around this
>> but
>> >>> >>>> > >> I updated all the tests instead
>> >>> >>>> > >> - Mock implementations no longer use the tuple as above
>> >>> >>>> > >> - We cannot support Python 3.2 officially anymore. Requests
>> >>> >>>> > >> does not support 3.2
>> >>> >>>> > >> - The raw connection still uses httplib. I decided it was too
>> >>> >>>> > >> risky to swap that for requests' method of uploading files.
>> >>> >>>> > >>
>> >>> >>>> > >>
>> https://github.com/apache/libcloud/pull/923#partial-pull-mergin
>> >>> >>>> > >> g
>> >>> >>>> > >>
>> >>> >>>> > >> Please remote fetch this branch and test it out on some
>> working
>> >>> >>>> > >> code talking to real APIs. Mocks can only go so far.
>> >>> >>>> > >>
>> >>> >>>> > >> I've uploaded the package here -
>> >>> >>>> > >> http://people.apache.org/~anthonyshaw/libcloud/1.5.0.post0/
>> >>> >>>> > >>
>> >>> >>>> > >> I would like to get this merged but would like some
>> additional
>> >>> >>>> > >> nods before it gets merged into trunk.
>> >>> >>>> > >>
>> >>> >>>> > >> Ant
>> >>> >>>> > >>
>> >>> >>>> >
>> >>> >>>>
>> >>> >
>> >>> >
>> >>> > NOTICE: This email and any attachments may contain confidential and
>> >>> proprietary information of NetSuite Inc. and is for the sole use of the
>> >>> intended recipient for the stated purpose. Any improper use or
>> distribution
>> >>> is prohibited. If you are not the intended recipient, please notify the
>> >>> sender; do not review, copy or distribute; and promptly delete or
>> destroy
>> >>> all transmitted information. Please note that all communications and
>> >>> information transmitted through this email system may be monitored by
>> >>> NetSuite or its agents and that all incoming email is automatically
>> scanned
>> >>> by a third party spam and filtering service
>> >>> >
>> >>> > </body></html>
>> >>>
>>

Reply via email to