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

fix: persist default provider deletion #1288

Merged
merged 1 commit into from
Nov 1, 2024

Conversation

idagelic
Copy link
Member

Persist default provider deletion

Description

Persists default provider/target deletion on Daytona Server restarting instead of redownloading every time

  • This change requires a documentation update
  • I have made corresponding changes to the documentation

Related Issue(s)

Closes #1273

@idagelic idagelic requested a review from a team as a code owner October 24, 2024 09:25
pkg/provider/manager/manager.go Outdated Show resolved Hide resolved
@idagelic idagelic force-pushed the enforce-default-provider-deletion branch from e448b03 to 55f64ab Compare October 24, 2024 13:34
lbrecic
lbrecic previously approved these changes Oct 25, 2024
@idagelic idagelic marked this pull request as draft October 25, 2024 13:09
@idagelic idagelic force-pushed the enforce-default-provider-deletion branch 2 times, most recently from 0c6aee7 to 5c6f0f1 Compare October 28, 2024 13:26
@idagelic idagelic marked this pull request as ready for review October 28, 2024 13:29
Copy link
Member

@Tpuljak Tpuljak left a comment

Choose a reason for hiding this comment

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

The logic seems a bit scattered to me.

I suggest the following:

  1. ProviderManager.DownloadProvider writes the lock file after downloading.
  2. ProviderManager.DownloadProvider has a skipIfLocked argument that will skip the download if the initial setup was locked - this will allow the controller install to redownload and for the server to skip

Seems to me that this will simplify things, especially in the Server which writes and checks lockfiles itself.

pkg/server/providers.go Outdated Show resolved Hide resolved
pkg/provider/manager/manager.go Show resolved Hide resolved
pkg/server/providers.go Outdated Show resolved Hide resolved
pkg/provider/manager/manager.go Outdated Show resolved Hide resolved
@idagelic
Copy link
Member Author

@Tpuljak DownloadProvider writing the lockfile would make it so that setting preset targets never gets executed.
DownloadProvider shouldn't have a "skip if locked" argument - DownloadProvider should download the provider and if we don't want to download we should do the checks outside of that function.

@Tpuljak
Copy link
Member

Tpuljak commented Oct 30, 2024

@Tpuljak DownloadProvider writing the lockfile would make it so that setting preset targets never gets executed. DownloadProvider shouldn't have a "skip if locked" argument - DownloadProvider should download the provider and if we don't want to download we should do the checks outside of that function.

Okay, lets at least go with this (#1288 (comment)) then.

@idagelic idagelic force-pushed the enforce-default-provider-deletion branch from 5c6f0f1 to 6fc261d Compare October 31, 2024 16:13
Copy link
Member

@Tpuljak Tpuljak left a comment

Choose a reason for hiding this comment

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

Nice work!

@idagelic idagelic merged commit 05aa014 into main Nov 1, 2024
12 checks passed
@idagelic idagelic deleted the enforce-default-provider-deletion branch November 1, 2024 09:00
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.

Restarting the server downloads default providers and sets preset targets
3 participants