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

Adds machine readable conninfo durations #4973

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ripienaar
Copy link
Contributor

The server use a home-grown encoding format for durations that's hard to use programatically while pleasing to look at - see myUptime() function.

This adds matching time.Duration based ones for machines.

Signed-off-by: R.I.Pienaar [email protected]

@ripienaar ripienaar requested a review from a team as a code owner January 18, 2024 12:09
Copy link
Member

@derekcollison derekcollison left a comment

Choose a reason for hiding this comment

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

Should this be opt-in? Should we consider the ability to form shapes by requesting only certain fields?

I am growing concerned watching SCP on NGS deal with the number of connections it already has, and thinking when then goes up multiple orders of magnitude.

Not against making it easier but concerned on longer term impact.

@ripienaar
Copy link
Contributor Author

Opt-in doesnt seem like a nice granular approach but for sure being able to extract subsets of fields would be handy.

@derekcollison
Copy link
Member

The string we use is directly parseable to a time.Duration, but that is in Go of course.

@ripienaar
Copy link
Contributor Author

The string we use is directly parseable to a time.Duration, but that is in Go of course.

if thats an API promise then I'll go with that :)

@derekcollison
Copy link
Member

I think the only place we deviate is in uptime since parse only supports up to hours IIRC. Would need to double check.

@derekcollison
Copy link
Member

I think we should close if clients can parse ok, as to not add additional weight here to the conninfo.

@ripienaar
Copy link
Contributor Author

OK I will verify it parses correctly then close

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.

2 participants