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.

Sign up for GitHub

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

Jump to bottom

Replace termination_policy and termination_idle_time with idle_duration: int|str|off #1154

Open
peterschmidt85 opened this issue Apr 20, 2024 · 11 comments · May be fixed by #2167
Open

Replace termination_policy and termination_idle_time with idle_duration: int|str|off #1154

peterschmidt85 opened this issue Apr 20, 2024 · 11 comments · May be fixed by #2167
Assignees
@peterschmidt85
Labels
major no-stale ux

Comments

@peterschmidt85
Copy link
Contributor

peterschmidt85 commented Apr 20, 2024 •
edited
Loading

I suggest that we drop termination_policy but rename termination_idle_time to idle_duration.
termination_idle_time is an important fieled but currently too long to be convenient to use. Also, I suggest that we use duration everywhere instead of somewhere time and somewhere duration.
@r4victor
Copy link
Collaborator

r4victor commented May 7, 2024

Looks good overall. Some notes:

  1. We're going to introduce new retry policies in Improve retry policy #1200 so retry_duration alone is not sufficient. Let's do not touch retry_policy in this issue.
  2. Regarding creation_policy, we don't have creation_policy: create. The addition of new policy should be discussed outside of renaming issue. Also, it's unclear what reuse: false|true|auto means. reuse: true may be misleading since it doesn't simply allows reuse but forces it. creation_policy: reuse|reuse-or-create is more clear. I'm against renaming this particular field as suggested.

@r4victor
Copy link
Collaborator

retry_policy renamed in #1200

@peterschmidt85
Copy link
Contributor Author

This issue is stale because it has been open for 30 days with no activity.

@peterschmidt85
Copy link
Contributor Author

This issue was closed because it has been inactive for 14 days since being marked as stale. Please reopen the issue if it is still relevant.

@peterschmidt85 peterschmidt85 closed this as not planned Won't fix, can't repro, duplicate, stale Jul 14, 2024
@peterschmidt85
Copy link
Contributor Author

We still need to rename termination_idle_time: int|str -> idle_duration: int|str|off

@github-actions GitHub Actions
Copy link

This issue is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale label Aug 25, 2024
@github-actions GitHub Actions
Copy link

This issue is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale label Sep 25, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 9, 2024

This issue is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale label Nov 9, 2024
@peterschmidt85
Copy link
Contributor Author

An update, here:

  1. If we agree to use fields like creation_policy, then I suggest that we rename retry back to retry_policy for consistency. Currently, the semantic of retry isn't 100% clear. Actually, it's a policy - it's an object that includes multiple fields.
  2. I suggest that we drop termination_policy but rename termination_idle_time to idle_duration.
    termination_idle_time is an important filed but currently too long to be convenient to use. Also, I suggest that we use duration everywhere instead of somewhere time and somewhere duration.

@github-actions github-actions bot removed the stale label Nov 11, 2024
@github-actions GitHub Actions
Copy link

This issue is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale label Dec 15, 2024
@github-actions GitHub Actions
Copy link

This issue was closed because it has been inactive for 14 days since being marked as stale. Please reopen the issue if it is still relevant.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Dec 30, 2024
@peterschmidt85 peterschmidt85 changed the title Rename some of the YAML properties to shorter names Replace termination_idle_time: int|str with idle_duration: int|str|off Dec 30, 2024
@peterschmidt85 peterschmidt85 self-assigned this Dec 30, 2024
peterschmidt85 added a commit that referenced this issue Dec 30, 2024
@peterschmidt85
Replace termination_idle_time: int|str with idle_duration: int|str|off …
9e69619 
…#1154
@peterschmidt85 peterschmidt85 linked a pull request Dec 30, 2024 that will close this issue
Replace termination_policy and termination_idle_time with idle_duration: int|str|off #2167
Open
@peterschmidt85 peterschmidt85 linked a pull request Dec 30, 2024 that will close this issue
Open
@peterschmidt85 peterschmidt85 changed the title Replace termination_idle_time: int|str with idle_duration: int|str|off Replace termination_policy and termination_idle_time with idle_duration: int|str|off Dec 30, 2024
peterschmidt85 added a commit that referenced this issue Dec 30, 2024
@peterschmidt85
Replace termination_idle_time: int|str with idle_duration: int|str|off …
63935fd 
…#1154
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@peterschmidt85 peterschmidt85

Labels
major no-stale ux
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Replace termination_policy and termination_idle_time with idle_duration: int|str|off dstackai/dstack
2 participants
@r4victor @peterschmidt85