Feature: Add File Upload in request form-data #231
Conversation
|
When will the version be released? |
|
Thanks for this. I'll try to review it tomorrow. |
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for file uploads in form-data by detecting file paths prefixed with "@" and dynamically overriding the Content-Type header.
- Introduces a loop that separates files from standard form-data using a defaultdict for values.
- Updates the header formation to exclude the "content-type" header and uses a dictionary comprehension instead of a list of tuples.
Comments suppressed due to low confidence (1)
src/posting/collection.py:267
- [nitpick] Switching from a list of tuples to a dictionary comprehension may drop duplicate headers with the same name. If multiple headers with the same name are expected, consider preserving the list format.
{ header.name: header.value for header in self.headers if header.enabled and header.name.lower() != "content-type" }
|
|
||
| if item.value.startswith("@"): | ||
| file_path = item.value[1:] | ||
| if not Path(file_path).exists(): |
There was a problem hiding this comment.
If the file does not exist, the code adds the value to data but then continues to open the file, which can lead to a runtime error. Consider adding an else clause or a continue statement to skip the file opening when the file is missing.
Copilot uses AI. Check for mistakes.
|
I previously considered how to add file parameters in posting, with the main ideas as follows:
|
|
I think I like the idea of using textual-autocomplete for this, and triggering the autocompletion via textual-autocomplete-4-path-completion__76pct_smaller.mp4I think the above would be a good extension to what's already in this PR :) |
|
For clarity - I do plan to merge and release this PR before adding the dropdown autocompletion. |
|
|
||
| if item.value.startswith("@"): | ||
| file_path = item.value[1:] | ||
| if not Path(file_path).exists(): |
There was a problem hiding this comment.
I'm undecided on whether the paths should be relative to the cwd or relative to the root of the collection directory. I'm concerned that if you were to share a collection, relative paths could break if they're cwd-relative.
There was a problem hiding this comment.
Based on my experience, developers rarely share files alongside collections, so I would suggest omitting the collection directory. Additionally, the current working directory (cwd) can vary between machines. What I’ve commonly seen from other clients is the use of a general working directory like ~/home/client_x. You can set that up and place all your files there for consistency.
That's not to say, you must always place your files there, you should always be able to set an absolute path.
src/posting/collection.py
Outdated
| [(header.name, header.value) for header in self.headers if header.enabled] | ||
| { | ||
| header.name: header.value | ||
| for header in self.headers | ||
| if header.enabled and header.name.lower() != "content-type" | ||
| } |
There was a problem hiding this comment.
A list of tuples were being used here because you can have multiple headers with the same name and different values in a request. For example:
Accept: text/html
Accept: application/xhtml+xml
Accept: application/xml;q=0.9
Addressing Issue: #224
@before the file path as convention form the curl -F optionMinor changes:
@in the start of a valueHope that is a quick and correct approach right now with the file upload part?
Let me know for any suggestions or improvements, thanks