- Notifications
You must be signed in to change notification settings - Fork928
web: use seconds in max TTL input#3576
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
b7fcf03 to92cf9a1Compare@BrunoQuaresma, I'm pretty new to TS and our frontend codebase. If I did anything in a suboptimal way, please let me know. |
Milliseconds are more difficult to deal with due toall of the zeros.Also, describe this feature as "auto-stop" to beconsistent with our Workspace page UI and CLI. "ttl"is our backend lingo which should eventually be updated.
I think we could do better, and set this as minutes, what do you think? |
@ammario I made the conversion in the handlers, so we don't have to add extra logic to map BE errors to a diff field, and vice-versa. |
@BrunoQuaresma — makes sense. Thanks for showing me how this is done. Plz approve and then it can merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'm all in favour of this! I think minutes is the lowest useful granularity of time with autostart though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I like this as well! Approving on behalf of FE to help you move this forward
I agree. Ideally the frontend accepts the same input as the CLI so there's so no truncation. I thought this PR was a step forward but not ideal. |
Milliseconds are more difficult to deal with due to
all of the zeros.