-
Notifications
You must be signed in to change notification settings - Fork 41
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
Feature/bodies #71
Feature/bodies #71
Conversation
this PR is the one I really care about ;). Or the stuff that starts from the first [requestbody] commit to be specific. |
Rebasing against master. |
I'll just wait until you're done then. Will this include the Fuchu changes as well? |
Yes |
New issue after rebase: #82 |
Rebase done. Afk for a while now. You should be able to test this. |
Not sure what's going on here, there seems to be some issue with the Fsharp.core reference in the unit tests, and the integration tests hang at some point. |
The fsharp.core redirect in the unit test project needs updating to 4.3.1. I'm getting two failures now, related to the multi-part file upload: formatting different sorts of body/can format multipart/formdata single file: Failed: --mACKqCcIID-J''_PL:hfbFiOLC/cew Larry Hello World --nLWsTCFurKCiU_PjC/cCmmU-tnJHHa Larry Hello World formatting different sorts of body/can format multipart/formdata with multipart/mixed for multi-file upload: Failed: --mACKqCcIID-J''_PL:hfbFiOLC/cew Larry --iDnsCZhfTqMSYsj:LhBTftNfVog:eA Hello World ...contents of file2.gif... --nLWsTCFurKCiU_PjC/cCmmU-tnJHHa Larry --BgOE:fCUQGnYfKwGMnxoyfwVMbRzZF Hello World ...contents of file2.gif... |
Also, the integration tests hang on this: ***** HttpClient_SuaveIntegrationTests+Suave Integration Tests.server receives valid filenames |
The first error is interesting; because it means Random(seed) has different implementations on mono vs .Net |
So it turns out RFC1867 that I coded against isn't actually implemented by web servers, and all web servers simply repeat the form-name for multiple files. Gaah. |
Wow, what a yak. At least now there are passing integration tests =). Ok, please review again. |
This way; if you use the typed API, you get the "auto-correct" Content-Type. If you use the BodyString (for e.g. JSON) or BodyRaw you get to set Content-Type by yourself like previously.
- open HttpClient -> open HttpFs.Client - Started moving rough edges out of Client.fs
Now we have integration tests for sending a binary file, too that are passing with the latest Suave. |
Latest suave https://twitter.com/SuaveIO/status/601018318280126464 -- works on my machine =) |
I have updated the namespaces here, too. |
So on Windows, on AppVeyor, something is obviously deadlocking / locking the async and that never returns. Since the web server runs to completion, sending all its data, I'm a bit surprised. Any insights? |
Some feedback now that all issues have been solved would be great! |
Good work, nice green builds! I'm due a 'learning day' at work, so I'll take that soon and try to go through all you've done. (Although there's a lot to go through!) |
This refactor closes almost all outstanding issues. - Introduces a Logging abstraction, fixes #80 - Previously: not version Releases -- fixes #78 - Previously: explicit state -- fixes #77 - Uses Map which has something like O(log n) -- fixes 76 - Username/password in UTF8 - fixes #73 - Fixes #72 by defaulting everything to UTF-8 - Fixes #67 by overwriting headers set n times - Previously: Fixes #65 about form bodies - Sets Expect100Continue to false, since we've overridden the GetResponseStream() interaction and always send all of the request. - Cleans up indexing into response.Headers to be functional instead of imperative - Improved docs on BodyForm
Request for merge! |
OK, have merged. Still want to check a few things, and want to go through it before making a NuGet. |
Yes, you should -- I absolutely want feedback and to reach consensus. How about we create issues for each discussion point, and you can cc @haf? |
A PR based on the previous two PRs, aiming to fix #65