New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
adds draft file upload example #19
Conversation
|
Your example immediately make it obvious we don't have async/await support for try await request.fileIO.writeFile(
contents: request.body,
path: fileURL.path,
context: request.context,
logger: request.logger
)
return "Uploaded as \(fileName)" Adding a download route as well would be cool |
@adam-fowler I was hoping for a minor tag on hummingbird/hummingbird so I can update the Package without using .branch("main"). |
I think this example is sufficiently simple. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to include a download options as well to round the example off.
If you could do the file upload/download via a web page served by the server that'd be even better
path: fileURL.path, | ||
context: request.context, | ||
logger: request.logger) | ||
return "Uploaded as \(fileName)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe return as JSON {"filename": "\(filename)"}
. You'd want to do that with the JSONEncoder
as well, so you get the correct content-type header, so set application.encoder = JSONEncoder()
and then return
struct UploadResponse: ResponseEncodable {
let filename: String
}
return UploadResponse(filename: "\(fileName)")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added
import HummingbirdFoundation | ||
|
||
extension HBRequest { | ||
var fileIO: HBFileIO { HBFileIO(application: self.application) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this really necessary? I'm sure it would be more obvious what you are doing if you just create an HBFileIO
in the route handler
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @mcritz I'm not seeing any of these changes
return "Uploaded as \(fileName)" | ||
} | ||
|
||
private var tempDirectory: URL = FileManager.default.temporaryDirectory |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it worth having this save to a local folder, so files are persistent between runs
@adam-fowler Sorry for the delays. The challenges of parenting and American holidays. I have updated presently. |
@mcritz dont worry. Especially at this time your family should be priority |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
There are a couple of minor typos in the readme
upload-async/README.md
Outdated
- In this example, we allow arbitrary uploads without a `File-Name` header and substitute a UUID instead. | ||
- In this example, we configure the route’s ability to overwrite an existing filename with the same filename. Developers should consider abstracting filenames from users entirely. A good practice is to assign UUID based filenames and store associated metadata in a database. | ||
|
||
Also noteworthy is that we’ve updaed the Application.configure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
Addresses PR Feedback
@adam-fowler Updated! |
A practical demo of how and why one may use async await.