Skip to content
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

3.0.4: Improved request-response example #3997

Merged
merged 1 commit into from
Aug 7, 2024

Conversation

ralfhandl
Copy link
Contributor

@ralfhandl ralfhandl commented Aug 5, 2024

  • separate code blocks for request and response message
  • adjusted content-length to what my editor claims it to be

Fixes #1720

@ralfhandl ralfhandl added the editorial Wording and stylistic issues label Aug 5, 2024
@ralfhandl ralfhandl added this to the v3.0.4 milestone Aug 5, 2024
@ralfhandl ralfhandl changed the title Improved request-response example 3.0.4: Improved request-response example Aug 5, 2024
Copy link
Member

@handrews handrews left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure I'm missing something here with the content-length, but I'm not sure what?

@@ -2036,17 +2036,21 @@ For example, given the following HTTP request:
POST /subscribe/myevent?queryUrl=https://clientdomain.com/stillrunning HTTP/1.1
Host: example.org
Content-Type: application/json
Content-Length: 187
Content-Length: 188
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused by both numbers- to me it looks like it's 260 if there is a trailing newline (more if the line ends are CRLF):

Python 3.8.13 (default, May 11 2022, 14:42:53) 
[Clang 13.1.6 (clang-1316.0.21.2.3)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> x = """{
...   "failedUrl" : "https://clientdomain.com/failed",
...   "successUrls" : [
...   "failedUrl": "https://clientdomain.com/failed",
...   "successUrls": [
...     "https://clientdomain.com/fast",
...     "https://clientdomain.com/medium",
...     "https://clientdomain.com/slow"
...   ]
... }
... """
>>> x
'{\n  "failedUrl" : "https://clientdomain.com/failed",\n  "successUrls" : [\n  "failedUrl": "https://clientdomain.com/failed",\n  "successUrls": [\n    "https://clientdomain.com/fast",\n    "https://clientdomain.com/medium",\n    "https://clientdomain.com/slow"\n  ]\n}\n'
>>> len(x)
260
>>> 

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Funny, I did something similar with Node.js and get 188:

Welcome to Node.js v22.5.1.
Type ".help" for more information.
> var text = `{
...   "failedUrl": "https://clientdomain.com/failed",
...   "successUrls": [
...     "https://clientdomain.com/fast",
...     "https://clientdomain.com/medium",
...     "https://clientdomain.com/slow"
...   ]
... }`
undefined
> text
'{\n' +
  '  "failedUrl": "https://clientdomain.com/failed",\n' +
  '  "successUrls": [\n' +
  '    "https://clientdomain.com/fast",\n' +
  '    "https://clientdomain.com/medium",\n' +
  '    "https://clientdomain.com/slow"\n' +
  '  ]\n' +
  '}'
> text.length
188

Which is identical to what my editor shows when I select that text block in the markdown source.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity I repeated the experiment with Python:

Python 3.9.6 (default, Feb  3 2024, 15:58:27) 
[Clang 15.0.0 (clang-1500.3.9.4)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> text="""{
...   "failedUrl": "https://clientdomain.com/failed",
...   "successUrls": [
...     "https://clientdomain.com/fast",
...     "https://clientdomain.com/medium",
...     "https://clientdomain.com/slow"
...   ]
... }"""
>>> text
'{\n  "failedUrl": "https://clientdomain.com/failed",\n  "successUrls": [\n    "https://clientdomain.com/fast",\n    "https://clientdomain.com/medium",\n    "https://clientdomain.com/slow"\n  ]\n}'
>>> len(text)
188

Which produces the same length of 188 as Node.js.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I see it: your experiment contains duplicated lines:
image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reproduced Ralf's experiment with the same result, so 188 does seem to be the right length. But I think the result of len(text) in Python is in units of unicode characters, which may not be the length of a UTF-8-encoded response body (though in this case it is). The way to get the length of a string encoded to UTF-8 in Python is

>>> len(text.encode('utf-8'))
188

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

188 it is! I swear I stared at that and could not figure out what was going on.... sorry to take up your time, folks!

@ralfhandl ralfhandl requested a review from handrews August 6, 2024 07:53
Copy link
Contributor

@mikekistler mikekistler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! 👍

@ralfhandl ralfhandl merged commit b7c20ee into OAI:v3.0.4-dev Aug 7, 2024
1 check passed
@ralfhandl ralfhandl deleted the 3.0.4/consistent-formatting branch August 7, 2024 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial Wording and stylistic issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants