Add a post method that allows sending binary data

[UVV-gh]

When the data to be sent comes from rather big file, it is more
efficient to read them from the file directly.

Providing data as a parameter to the exising post method could cause
extremely high memory usage.

Signed-off-by: Vyacheslav Yurkov Vyacheslav.Yurkov@bruker.com

[UVV-gh]

There are more details on the issue provided in this SO post https://stackoverflow.com/questions/54907281/robot-framework-send-binary-data-in-post-request-body-with

[lucagiove]

I don't know much about this part but isn't the files parameter works already in this way?
https://2.python-requests.org/en/master/user/quickstart/#post-a-multipart-encoded-file

[UVV-gh]

@lucagiove I assume that would be a different beast. The SO post explains the issue clearly. That would be a part of relevant request's documentation https://2.python-requests.org/en/master/user/advanced/#streaming-uploads

[lucagiove]

Sorry I missed to read the Stack Overflow I'll have a read.

[UVV-gh]

Looks like the patch is bit old now. I take a look at rebasing it...

[lucagiove]

Before working on it let me see if it's a feature that I would merge and in case how could be properly integrated I'll let you know soon.
There are some plan to change a lot the structure of the keywords, see the open feature issues.

[lucagiove]

Hi, I had a look at original requests library code when files parameter is passed and seems to open it as a file descriptor stream.
In any case another option to allow passing a file descriptor as data could be to create a keyword that returns a file descriptor. eg Get File Descriptor (this to avoid loading the whole file in memory as you mentioned on stack overflow) and use the standard requests keywords.
What do you think?

[UVV-gh]

Alright. I think that makes sense. Let me rebase onto latest master and update my pull request according to your suggestions.

[lucagiove]

Gr8 remind to add tests and documentation!

[UVV-gh]

A few tests got broken I'm working on them..

Now I remember why I added a separate method for 'binary' post. The downsides are:

• The 'data' variable has to be checked in a few places if it's a file handler
• Without "with" the file handler effectively remains open until the end of the test (perhaps even until the end of all tests, I'm not sure when it gets released)

I hope these limitations are OK. I let you know when the tests are fixed

[UVV-gh]

I could, of course, add another keyword for closing a file handler.. not sure if it fits the design though :)

Another thought is to close a handle in post() method. But that doesn't look good too.

[UVV-gh]

Alright, I went the second way and close a handle right after POST request. Not sure what happens if exception is thrown on the way though.

The tests run. Please take a look and let me know what you think.

[lucagiove]

Hi! Thank you for your work!

About problem of multiple check of data: in the near future I want to remove all the 'data' parsing and pass directly to requests library, so this should be covered.
You can see my work in progress idea in this branch:
https://github.com/bulkan/robotframework-requests/tree/new-keywords
We can say that the file descriptor feature would work only for the "GET/POST./... On Session" keywords, I want to deprecate all the others.

Instead regarding the open file you're right since the library is global I think file descriptor would not be closed automatically (except if requests does it automatically).

[lucagiove]

I didn't check yet the code I was just answering to the previous comments.
Thanks I'll have a look to the PR soon hopefully.

[lucagiove]

I think the approach is valid, some minor changes and can be integrated.

[UVV-gh]

Updated according to the comments

[lucagiove]

if you give me write access to the branch I'd like to add some small changes before merging

[UVV-gh]

Never did that before. Sent you an Email. Please check if that would be enough

[lucagiove]

@UVV-gh I was wandering how to explain the difference between using the file descriptor and files parameter in the documentation of the keywords.
You mentioned something about the multipart.

[UVV-gh]

I think multipart is related to "files" only. File descriptor is used for continuous data stream. Probably makes sense just to refer to official requests documentation, doesn't it?

[lucagiove]

Before merging I want to do a test comparing file descriptor and files usage.

[lucagiove]

Before merging I want to do a test comparing file descriptor and files usage.

Not needed, trying to rename and add more doc.

[lucagiove]

Renamed Get File Descriptor to Get File For Streaming Upload so that it's more clear what it has been made for instead of what it does.
@UVV-gh what do you think?
Updated also the doc to make clear the usage, maybe an example in the doc might help?

[UVV-gh]

Looks fine for me. As for the docs, it's up to you. Looking briefly through the documentation, I didn't find much of the description of other features. On the other hand, unit tests cover most of the use cases ;)

[shadeimi]

Manage OSError exception on missing file