Skip to content

Conversation

@dom96
Copy link
Contributor

@dom96 dom96 commented Feb 22, 2020

As discussed in #13394, these changes cannot work. Reverted via

git revert --no-commit 5bf571f061d53d35aab727f420afd9f415987723
git revert --no-commit abd660c407d00d0c4f2129ff11bfc69badda8ece
git revert --no-commit 955465e5f42b1353f69f3bd884908a7ef91ce13b
git commit

As discussed in #13394, these changes cannot work. Reverted via

```
git revert --no-commit 5bf571f
git revert --no-commit abd660c
git revert --no-commit 955465e
git commit
```
@Araq
Copy link
Member

Araq commented Feb 22, 2020

Pinq @mrhdias. As I said elsewhere, better than reverting this is fixing the feature so that it works well. It's an important feature.

@mrhdias
Copy link
Contributor

mrhdias commented Feb 23, 2020

Pinq @mrhdias. As I said elsewhere, better than reverting this is fixing the feature so that it works well. It's an important feature.

Thank you @Araq. The idea of using an iterator is not new.
#6576

I replace "let data = request.client.recv(readSize)" with "let data = waitFor request.client.recv(readSize)" to wait for the data.

So, reading the data is clear:

for data in req.bodyStream(8*1024):
  echo data

Iterator code

when (NimMajor, NimMinor) >= (1, 0):
  iterator bodyStream*(
    request: Request,
    chunkSize: int = 8*1024): string =
    ## The chunkSize parameter is optional and default value is 8*1024 bytes.
    ##
    ## This iterator return a tuple with the length of the data that was read
    ## and a future.
    var remainder = request.contentLength
    while remainder > 0:
      let readSize = min(remainder, chunkSize)
      let data = waitFor request.client.recv(readSize)
      if data.len != readSize:
        raise newException(ValueError, "Error reading POST data from client.")
      yield data
      remainder -= readSize

@mrhdias
Copy link
Contributor

mrhdias commented Feb 23, 2020

Another solution forum (Thanks Hlaaftana): https://forum.nim-lang.org/t/5971

template forStream(req: Request, chunkSize: int, name, body: untyped): untyped =
  var remainder = req.contentLength
  while true:
    let `name` = await req.client.recv(min(remainder, chunkSize))
    body
    remainder -= len(`name`)
    if remainder == 0:
      break

req.forStream(8*1024, d):
  echo d

@dom96
Copy link
Contributor Author

dom96 commented Feb 23, 2020

It is better to fix this, but it's going to be much easier to review if we start fresh IMO. Can we merge this?

@dom96
Copy link
Contributor Author

dom96 commented Mar 6, 2020

Okay, I am merging this since there was no feedback in 12 days. Please ping me to review any new PRs on this.

@dom96 dom96 merged commit ec8a17c into devel Mar 6, 2020
@narimiran narimiran deleted the revert-asynchttpserver-changes branch March 9, 2020 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants