search

henon / Python.Included

A Python.NET based framework enabling .NET libraries to call into Python packages without depending on a local Python installation.
MIT License
313 stars 51 forks source link

Using httpclient for downloads to remove reliance on curl #43

Closed keg247 closed 2 years ago

keg247 commented 2 years ago

I'm not sure if you had a specific reason to rely on curl, but I wanted to remove the dependency so I implemented a downloader that uses httpclient for the downloading and that can report progress on the download. Unfortunately, doing so results in changing the signature of a number of public methods to async. That seems to be a breaking change. In any event, obviously feel free to accept if you find it useful or reject.

henon commented 2 years ago

I like it and I'll just increase the major version to release this due to the breaking API change. I take it you tested this well?

keg247 commented 2 years ago

I had tested it using a console app and everything was working well, but I went ahead and added a few tests for the downloader. Unfortunately, they are not really "unit" tests because of the difficulty of testing with HttpClient in a static class (I can't create a fake HttpMessageHandler and instantiate an HttpClient with that), so the tests use a live HttpClient that fetches the python_included_nuget.png file in your repository. The tests are therefore a bit fragile, but should suffice. While testing, I realized that the outputfile is not deleted if the download is canceled, so the new commits also handle that, as well as adding some additional awaits because of the changed signatures.

henon commented 1 year ago

I know, a bit late, but your change has finally been released: https://www.nuget.org/packages/Python.Deployment/2.0.0

keg247 commented 1 year ago

That's great! Thanks for letting me know.