Skip to content
This repository was archived by the owner on Oct 12, 2023. It is now read-only.

update max user agent length.#42

Merged
ramya-rao-a merged 5 commits into
Azure:masterfrom
ShivangiReja:userAgentLenth
Apr 16, 2019
Merged

update max user agent length.#42
ramya-rao-a merged 5 commits into
Azure:masterfrom
ShivangiReja:userAgentLenth

Conversation

@ShivangiReja
Copy link
Copy Markdown
Member

Description

Brief description of the changes made in the PR. This helps in making better change log

  • There's a hard limit of 128 characters on the user agent string and it could easily go over the limit( custom user agent string etc).
  • This pr updates the limit to 512 characters.

Reference to any github issues

@ShivangiReja ShivangiReja added the Client Issues that refer to the client sdk label Apr 15, 2019
@ShivangiReja ShivangiReja self-assigned this Apr 15, 2019
Comment thread lib/ConnectionContextBase.ts Outdated
if (userAgent.length > Constants.maxUserAgentLength) {
throw new Error(
`The user-agent string cannot be more than 128 characters in length.` +
`The user-agent string cannot be more than 512 characters in length.` +
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Lets use the constant maxUserAgentLength here instead of hard coding 512.

Copy link
Copy Markdown
Contributor

@ramya-rao-a ramya-rao-a left a comment

Choose a reason for hiding this comment

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

Right after creating the connectionOptions object lets use log to log the same.

Copy link
Copy Markdown
Contributor

@ramya-rao-a ramya-rao-a left a comment

Choose a reason for hiding this comment

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

Let's also update the version number so we can release the next version today

@ShivangiReja
Copy link
Copy Markdown
Member Author

@ramya-rao-a As discussed offline we will add a log for all the connection properties after successfully creating a connection in EventHubs.

@ramya-rao-a
Copy link
Copy Markdown
Contributor

@ShivangiReja Azure/azure-sdk-for-js#2270 was the fix done in the main repo to fix the auditing issue.
Please do the same here to get the CI check

@mikeharder
Copy link
Copy Markdown
Member

mikeharder commented Apr 16, 2019

@ramya-rao-a , @ShivangiReja : PR to update dependencies: #43

@ramya-rao-a ramya-rao-a merged commit 3892d07 into Azure:master Apr 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Client Issues that refer to the client sdk

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants