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 data directory excluding sessions to dump (#586) #587

Merged
merged 1 commit into from
Jan 12, 2017
Merged

Add data directory excluding sessions to dump (#586) #587

merged 1 commit into from
Jan 12, 2017

Conversation

mkuhlmann
Copy link
Contributor

This includes the data directory inside a gitea dump. Sessions are excluded if the session provider is set to file and the session directory is a child of the data directory.

Closes #586

@lunny lunny added the type/bug label Jan 5, 2017
@lunny lunny added this to the 1.1.0 milestone Jan 5, 2017
@lunny
Copy link
Member

lunny commented Jan 5, 2017

sessions dir is still in the data

@tboerger tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 5, 2017
@mkuhlmann
Copy link
Contributor Author

Works now. The issue was an unset PROVIDER_CONFIG in custom app.ini. Then the configuration value is just empty and macaron uses a standard data/sessions. Thats another bug that should be resolved, since the expected behavior should session path to be relative to APP_DATA_PATH and not to the executable path. If APP_DATA_PATH is set to another location but PROVIDER_CONFIG is not set, sessions will be still saved to data/sessions.

I've hardcoded data/sessions to match macarons fallback config var as a workaround and it works for now.

@lunny
Copy link
Member

lunny commented Jan 5, 2017

LGTM

@tboerger tboerger added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jan 5, 2017
bkcsoft
bkcsoft suggested changes Jan 6, 2017
View reviewed changes
Copy link
Member

@bkcsoft bkcsoft left a comment

Choose a reason for hiding this comment

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

Please don't use uncommon abbreviations for variables :)

cmd/dump.go
// Make sure we have all upper level directories.
z.AddEmptyDir(dirPath)

fis, err := dir.Readdir(0)
Copy link
Member

Choose a reason for hiding this comment

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

could we call it files, err instead?

cmd/dump.go
if err != nil {
return err
}
for _, fi := range fis {
Copy link
Member

Choose a reason for hiding this comment

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

same hear, file := range files

@mkuhlmann
Copy link
Contributor Author

@bkcsoft Fixed, much cleaner now ;)

@strk
Copy link
Member

strk commented Jan 7, 2017

LGTM

@tboerger tboerger added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jan 7, 2017
@lunny lunny merged commit e698654 into go-gitea:master Jan 12, 2017
@lunny
Copy link
Member

lunny commented Jan 12, 2017

We have to merge this at first. @bkcsoft if you have any further information, please open another issue.

@mkuhlmann mkuhlmann deleted the dump-with-data branch January 12, 2017 13:41
lunny pushed a commit to lunny/gitea that referenced this pull request Feb 16, 2017
@mkuhlmann @lunny
Add data directory excluding sessions to dump (go-gitea#587)
54750b2
@lunny lunny mentioned this pull request Feb 16, 2017
Merged
lunny added a commit that referenced this pull request Feb 16, 2017
@lunny
Add data directory excluding sessions to dump (#587) (#959)
5fcf218
@lunny lunny added the backport/done All backports for this PR have been created label Feb 16, 2017
@go-gitea go-gitea locked and limited conversation to collaborators Nov 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bkcsoft bkcsoft bkcsoft requested changes

Assignees
No one assigned
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug
Projects
None yet
Milestone
1.1.0
Development

Successfully merging this pull request may close these issues.

Dump should contains data directory except sessions
5 participants
@mkuhlmann @lunny @strk @bkcsoft @tboerger