Skip to content
This repository was archived by the owner on Feb 18, 2025. It is now read-only.

Format delays in days / hours / minutes / seconds #1184

Merged
merged 2 commits into from
Jun 6, 2020

Conversation

sjmudd
Copy link
Collaborator

@sjmudd sjmudd commented Jun 4, 2020

Delays are shown in seconds which is fine but often you want to see this in days / hours / minutes and seconds without doing mental arithmetic to figure out the delay.

This patch shows time since a box has been seen and replication delay in days / hours / minutes and seconds and thus saves time with a large delay in seeing what it represents.

I'll include a couple of examples with the new formatting.
screenshot-1
screenshot-2
screenshot-3

I'm exposing lots of warts :-)

@sjmudd
Copy link
Collaborator Author

sjmudd commented Jun 4, 2020

cc: @luisyonaldo @dveeden

@koorgoo
Copy link

koorgoo commented Jun 4, 2020

Did you insert spaces between units intentionally, i.e. 1d 2h 3m 0s?
Don't you want the function to work similar to https://golang.org/pkg/time/#Duration.String and return 1d2h3m0s?

@dveeden
Copy link
Contributor

dveeden commented Jun 4, 2020

Wouldn't there be some JS function that does what formattedInterval() does?

Otherwise: Useful functionality and code looks good to me, but didn't test it.

side note: You know that instead of editing screenshots you can get orchestrator to mask hostnames?

@dveeden
Copy link
Contributor

dveeden commented Jun 4, 2020

Wouldn't there be some JS function that does what formattedInterval() does?

I've searched and couldn't find anything that's obviously better.

@shlomi-noach
Copy link
Collaborator

shlomi-noach commented Jun 4, 2020 via email

@koorgoo
Copy link

koorgoo commented Jun 4, 2020

But we should use golang's existing formatting.

Then we should use only hours, minutes, and seconds.

And maybe rename formattedInterval() to durationString().

@sjmudd
Copy link
Collaborator Author

sjmudd commented Jun 4, 2020

To answer the questions:

  • spaces are intentional. I think they read better. YMMV.
  • I have no clue about javascript so am quite happy to be provided a better solution. I just want to see this using a more human readable output.
  • I am just sick of seeing unformatted seconds.
  • the patch seemed reasonable and I'm sure we can improve it later but as the first attempt this looks reasonable.
  • Dima if you don't like the name I can change it.

@koorgoo
Copy link

koorgoo commented Jun 4, 2020

The current implementation already looks good to me 👌

@shlomi-noach shlomi-noach merged commit bb42ff7 into openark:master Jun 6, 2020
@sjmudd sjmudd deleted the sjmudd/improve_delay_formatting branch December 25, 2021 20:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants