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

Add --skip-db option to dump command #30613

Merged
merged 6 commits into from
Apr 21, 2024
Merged

Add --skip-db option to dump command #30613

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f6f665c
Add --skip-db option to dump cmd
kemzeb Apr 20, 2024
583d48e
Merge branch 'main' into cmd-dump-add-skip-db-option
GiteaBot Apr 21, 2024
13163ee
Merge branch 'main' into cmd-dump-add-skip-db-option
GiteaBot Apr 21, 2024
3719990
Merge branch 'main' into cmd-dump-add-skip-db-option
GiteaBot Apr 21, 2024
cccbd14
Merge branch 'main' into cmd-dump-add-skip-db-option
GiteaBot Apr 21, 2024
c8f8776
Merge branch 'main' into cmd-dump-add-skip-db-option
GiteaBot Apr 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
58 changes: 34 additions & 24 deletions cmd/dump.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ var CmdDump = &cli.Command{
Name: "skip-index",
Usage: "Skip bleve index data",
},
&cli.BoolFlag{
Name: "skip-db",
Usage: "Skip database",
},
&cli.StringFlag{
Name: "type",
Usage: fmt.Sprintf(`Dump output format, default to "zip", supported types: %s`, strings.Join(dump.SupportedOutputTypes, ", ")),
Expand Down Expand Up @@ -185,35 +189,41 @@ func runDump(ctx *cli.Context) error {
}
}

tmpDir := ctx.String("tempdir")
if _, err := os.Stat(tmpDir); os.IsNotExist(err) {
fatal("Path does not exist: %s", tmpDir)
}
if ctx.Bool("skip-db") {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't add a comment to the specific code snippet for some reason, but should we avoid firing up the xorm engine if we are skipping the database (so lines 140-146 as of the current snapshot of my branch). After looking around I think we can add it into my else branch but I'm unsure if it is needed anywhere else in the function.

// Ensure that we don't dump the database file that may reside in setting.AppDataPath or elsewhere.
dumper.GlobalExcludeAbsPath(setting.Database.Path)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it safe to assume that setting.Database.Path is an absolute path?

Copy link
Contributor Author

@kemzeb kemzeb Apr 20, 2024 •
edited

Choose a reason for hiding this comment

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

Also, I seen other areas that perform a ctx.IsSet() before a ctx.Bool(), but after reading the docs associated to the Bool() method it should return false if --skip-db is not provided; so I decided to only use this function. Please let me know if this is a problem

Copy link
Member

Choose a reason for hiding this comment

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

But the path maybe empty? Whether it will affect the result?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From looking at GlobalExcludeAbsPath() (see here), if it was passed the empty string, it would essentially append "." to dumper.globalExcludeAbsPaths.

Even though it's not an absolute path, it doesn't look like it would affect the result as we if we add a "." to this slice any subsequent paths that are just the empty string would also be cleaned to be "." and therefore be excluded since that string is in the slice. If you feel differently, please let me know and I'll add an empty string check for this

log.Info("Skipping database")
} else {
tmpDir := ctx.String("tempdir")
if _, err := os.Stat(tmpDir); os.IsNotExist(err) {
fatal("Path does not exist: %s", tmpDir)
}

dbDump, err := os.CreateTemp(tmpDir, "gitea-db.sql")
if err != nil {
fatal("Failed to create tmp file: %v", err)
}
defer func() {
_ = dbDump.Close()
if err := util.Remove(dbDump.Name()); err != nil {
log.Warn("Unable to remove temporary file: %s: Error: %v", dbDump.Name(), err)
dbDump, err := os.CreateTemp(tmpDir, "gitea-db.sql")
if err != nil {
fatal("Failed to create tmp file: %v", err)
}
}()
defer func() {
_ = dbDump.Close()
if err := util.Remove(dbDump.Name()); err != nil {
log.Warn("Unable to remove temporary file: %s: Error: %v", dbDump.Name(), err)
}
}()

targetDBType := ctx.String("database")
if len(targetDBType) > 0 && targetDBType != setting.Database.Type.String() {
log.Info("Dumping database %s => %s...", setting.Database.Type, targetDBType)
} else {
log.Info("Dumping database...")
}
targetDBType := ctx.String("database")
if len(targetDBType) > 0 && targetDBType != setting.Database.Type.String() {
log.Info("Dumping database %s => %s...", setting.Database.Type, targetDBType)
} else {
log.Info("Dumping database...")
}

if err := db.DumpDatabase(dbDump.Name(), targetDBType); err != nil {
fatal("Failed to dump database: %v", err)
}
if err := db.DumpDatabase(dbDump.Name(), targetDBType); err != nil {
fatal("Failed to dump database: %v", err)
}

if err = dumper.AddFile("gitea-db.sql", dbDump.Name()); err != nil {
fatal("Failed to include gitea-db.sql: %v", err)
if err = dumper.AddFile("gitea-db.sql", dbDump.Name()); err != nil {
fatal("Failed to include gitea-db.sql: %v", err)
}
}

log.Info("Adding custom configuration file from %s", setting.CustomConf)
Expand Down