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

Change paths to use woodpecker instead of drone #494

Merged
merged 3 commits into from
Oct 28, 2021

Conversation

anbraten
Copy link
Member

@anbraten anbraten commented Oct 28, 2021

Major part of this PR has been implemented by @jolheiser in #438 and #431

cmd/server/setup.go Outdated Show resolved Hide resolved
Copy link
Member

@jolheiser jolheiser left a comment

Choose a reason for hiding this comment

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

I think this looks fine. Note that it closes my other two PRs because we had split up the path change and sqlite change.

@anbraten
Copy link
Member Author

I think as the base path change changes the path of the sqlite file as well it is fine that we keep both changes in one PR. If you want, feel free to update your PR to get credits 😉

@jolheiser jolheiser merged commit 06800cb into woodpecker-ci:master Oct 28, 2021
@anbraten anbraten deleted the change-path branch October 28, 2021 20:39
@6543 6543 added this to the 0.15.0 milestone Oct 29, 2021
@6543 6543 added breaking will break existing installations if no manual action happens refactor delete or replace old code labels Oct 29, 2021
if c.String("datasource") == "/var/lib/woodpecker/woodpecker.sqlite" {
_, err := os.Stat("/var/lib/drone/drone.sqlite")
if err == nil {
return os.Rename("/var/lib/drone/drone.sqlite", "/var/lib/woodpecker/woodpecker.sqlite")
Copy link
Member

Choose a reason for hiding this comment

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

this is not the best sol. since if you mout volume at /var/lib/drone it will move the DB outside of the volume!!

Copy link
Member

Choose a reason for hiding this comment

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

Instead of renaming I would propose to detect on driver=sqlite3 where the file could be on following order:

  1. /var/lib/woodpecker/woodpecker.sqlite -> All fine
  2. /var/lib/woodpecker/drone.sqlite -> rename to /var/lib/woodpecker/woodpecker.sqlite (print a warining(renamed ...))
  3. /var/lib/drone/drone.sqlite -> do nothing just use it and print a error!

6543 added a commit that referenced this pull request Oct 30, 2021
#494 introduced a bug, where a migration function can remove the sqlite3 file outside of the mounted docker volume.
that would result in a data lose after a container recreate.

this fix it by only rename the file if in same folder else just use the old path as fallback and put warnings into the log

Co-authored-by: Anbraten <anton@ju60.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking will break existing installations if no manual action happens refactor delete or replace old code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants