-
Notifications
You must be signed in to change notification settings - Fork 885
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
b7fcf03
to
92cf9a1
Compare
@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 to all of the zeros. Also, describe this feature as "auto-stop" to be consistent 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.