feat: Add support for uploading files as a list #403

vanhanit · 2024-10-09T10:05:21Z

Uploading a list of files with the same key is supported by the Requests library. This allows the receiving server to accept a list of files for the same key, i.e. accepting any number of files.

Issue: #401

vanhanit · 2024-10-09T11:15:39Z

This could also be a candidate for a back-port to the 0.9 version as well.

codecov · 2024-10-25T13:16:26Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.86%. Comparing base (1e34855) to head (0fadd9f).
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #403      +/-   ##
==========================================
+ Coverage   85.07%   88.86%   +3.79%     
==========================================
  Files           9        9              
  Lines         469      476       +7     
  Branches       47       49       +2     
==========================================
+ Hits          399      423      +24     
+ Misses         65       51      -14     
+ Partials        5        2       -3

Flag	Coverage Δ
acceptance	`85.29% <100.00%> (+0.21%)`	⬆️
unit	`60.08% <80.00%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

lucagiove

I would also add a unit test to make sure files get closed for both cases list or tuple.
Thanks!

lucagiove · 2024-10-25T13:20:44Z

atests/http_server/helpers.py

-            files[k].append(val)
-        else:
-            files[k] = val
+    for k in request.files.keys():


you also had to fix httpbin?

Yes, as this only returned one item the way it was written. According to what I found out, you need to use the getlist(key) method to get all files for one key.

lucagiove · 2024-10-25T13:27:05Z

src/RequestsLibrary/RequestsKeywords.py

-        files_descriptor_to_close = filter(
-            is_file_descriptor, list(files.values()) + [data]
-        )
+


at this point I would extract the logic to close file descriptor in a dedicated function and try to cleanup this part

I refactored the closing of file descriptors into its own static method.

vanhanit · 2024-10-30T19:23:57Z

Added verification to the Robot Framework tests to verify that the file descriptors are closed for both use cases.

Uploading a list of files with the same key is supported by the Requests library. This allows the receiving server to accept a list of files for the same key, i.e. accepting any number of files. Issue: MarketSquare#401

lucagiove · 2024-10-30T20:02:31Z

Added verification to the Robot Framework tests to verify that the file descriptors are closed for both use cases.

there was already a unit test for check file closing

lucagiove · 2024-10-30T20:04:11Z

src/RequestsLibrary/RequestsKeywords.py

@@ -176,7 +192,7 @@ def session_less_get(
        | ``json``     | A JSON serializable Python object to send in the body of the request. |
        | ``headers``  | Dictionary of HTTP Headers to send with the request. |
        | ``cookies``  | Dict or CookieJar object to send with the request. |
-        | ``files``    | Dictionary of file-like-objects (or ``{'name': file-tuple}``) for multipart encoding upload. ``file-tuple`` can be a 2-tuple ``('filename', fileobj)``, 3-tuple ``('filename', fileobj, 'content_type')`` or a 4-tuple ``('filename', fileobj, 'content_type', custom_headers)``, where ``'content-type'`` is a string defining the content type of the given file and ``custom_headers`` a dict-like object containing additional headers to add for the file. |
+        | ``files``    | Dictionary of file-like-objects (or ``{'name': file-tuple}``) for multipart encoding upload. ``file-tuple`` can be a 2-tuple ``('filename', fileobj)``, 3-tuple ``('filename', fileobj, 'content_type')`` or a 4-tuple ``('filename', fileobj, 'content_type', custom_headers)``, where ``'content-type'`` is a string defining the content type of the given file and ``custom_headers`` a dict-like object containing additional headers to add for the file. List or tuple of ``('key': file-tuple)`` allows uploading multiple files with the same key, resulting in a list of files on the receiving end. |


the strange thing is that even the official documentation don't mention files as list or tuple..
https://requests.readthedocs.io/en/latest/api/#requests.request

Yes, I know that they don't mention it. But the support is there and I don't know how long it has been there.

vanhanit · 2024-10-30T20:23:40Z

Added verification to the Robot Framework tests to verify that the file descriptors are closed for both use cases.

there was already a unit test for check file closing

Well, I did not dig too deep to find that.
The Robot test ensures that both a dictionary and a list is tested.
It does not test having files in the data section. Maybe the unit test covers this?

vanhanit · 2024-10-30T20:24:59Z

I rebased on master. I don't think that my changes is what is causing the test to fail.

lucagiove · 2024-10-31T08:50:04Z

I rebased on master. I don't think that my changes is what is causing the test to fail.

no it's not your commit, there are still some tests that use a web version of httpbin, sometimes they throttle the requests...
they should be changed but there are tests also for https certificates so not easy to implement locally

vanhanit · 2024-10-31T09:13:39Z

I rebased on master. I don't think that my changes is what is causing the test to fail.

no it's not your commit, there are still some tests that use a web version of httpbin, sometimes they throttle the requests... they should be changed but there are tests also for https certificates so not easy to implement locally

Yeah, it is the same test that also fails locally for me.

vanhanit · 2024-11-04T08:14:55Z

@lucagiove Was there anything I still needed to do with this? How do we rerun the builds?

lucagiove

No I would have added also the unit test but mandatory.
I'll try to have the pipeline running.

vanhanit · 2024-11-04T11:01:08Z

No I would have added also the unit test but mandatory. I'll try to have the pipeline running.

But didn't you say that there already are a unit test for the closing of file descriptors? Or was there some other unit test that you wanted to run?
Either way, there are a couple of Robot Framework tests that cover the functionality.

lucagiove · 2024-11-04T11:02:53Z

No I would have added also the unit test but mandatory. I'll try to have the pipeline running.

But didn't you say that there already are a unit test for the closing of file descriptors? Or was there some other unit test that you wanted to run? Either way, there are a couple of Robot Framework tests that cover the functionality.

Yep but not with the specific input of a list.

lucagiove requested changes Oct 25, 2024

View reviewed changes

lucagiove linked an issue Oct 25, 2024 that may be closed by this pull request

Files param doesn't have to be a dictionary type #401

Closed

lucagiove assigned vanhanit Oct 25, 2024

vanhanit force-pushed the feature/add-files-as-list-support branch from 54da020 to d099ade Compare October 30, 2024 19:20

feat: Add support for uploading files as a list

0fadd9f

Uploading a list of files with the same key is supported by the Requests library. This allows the receiving server to accept a list of files for the same key, i.e. accepting any number of files. Issue: MarketSquare#401

vanhanit force-pushed the feature/add-files-as-list-support branch from d099ade to 0fadd9f Compare October 30, 2024 19:31

vanhanit requested a review from lucagiove October 30, 2024 19:32

lucagiove reviewed Oct 30, 2024

View reviewed changes

lucagiove approved these changes Nov 4, 2024

View reviewed changes

lucagiove merged commit c89c38b into MarketSquare:master Nov 4, 2024
8 checks passed

vanhanit deleted the feature/add-files-as-list-support branch November 5, 2024 07:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add support for uploading files as a list #403

feat: Add support for uploading files as a list #403

vanhanit commented Oct 9, 2024

vanhanit commented Oct 9, 2024

codecov bot commented Oct 25, 2024 •

edited

Loading

lucagiove left a comment

lucagiove Oct 25, 2024

vanhanit Oct 25, 2024

lucagiove Oct 25, 2024

vanhanit Oct 30, 2024

vanhanit commented Oct 30, 2024

lucagiove commented Oct 30, 2024

lucagiove Oct 30, 2024

vanhanit Oct 30, 2024

vanhanit commented Oct 30, 2024

vanhanit commented Oct 30, 2024

lucagiove commented Oct 31, 2024

vanhanit commented Oct 31, 2024

vanhanit commented Nov 4, 2024

lucagiove left a comment

vanhanit commented Nov 4, 2024

lucagiove commented Nov 4, 2024

feat: Add support for uploading files as a list #403

feat: Add support for uploading files as a list #403

Conversation

vanhanit commented Oct 9, 2024

vanhanit commented Oct 9, 2024

codecov bot commented Oct 25, 2024 • edited Loading

Codecov Report

lucagiove left a comment

Choose a reason for hiding this comment

lucagiove Oct 25, 2024

Choose a reason for hiding this comment

vanhanit Oct 25, 2024

Choose a reason for hiding this comment

lucagiove Oct 25, 2024

Choose a reason for hiding this comment

vanhanit Oct 30, 2024

Choose a reason for hiding this comment

vanhanit commented Oct 30, 2024

lucagiove commented Oct 30, 2024

lucagiove Oct 30, 2024

Choose a reason for hiding this comment

vanhanit Oct 30, 2024

Choose a reason for hiding this comment

vanhanit commented Oct 30, 2024

vanhanit commented Oct 30, 2024

lucagiove commented Oct 31, 2024

vanhanit commented Oct 31, 2024

vanhanit commented Nov 4, 2024

lucagiove left a comment

Choose a reason for hiding this comment

vanhanit commented Nov 4, 2024

lucagiove commented Nov 4, 2024

codecov bot commented Oct 25, 2024 •

edited

Loading