Skip to content

[docker] Separate various /data/gitea #6652

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

Closed
wants to merge 5 commits into from

Conversation

sapk
Copy link
Member

@sapk sapk commented Apr 16, 2019

This PR change nothing to the current used docker paths.
The goal is to distinguish paths (AppWorkPath, AppDataPath, CustomPath) that could be confusing based on default path of gitea.
It permit to detail what /data/gitea path we are using at each use.

Later we could revert to a more default path based on gitea.
Ex: GITEA_CUSTOM = WorkDir + "/custom"
But permitting to continue to use old paths by simply overriding the env variable.

And move the config of env variable in a dedicate file.

@codecov-io
Copy link

codecov-io commented Apr 16, 2019

Codecov Report

Merging #6652 into master will increase coverage by 0.22%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6652      +/-   ##
==========================================
+ Coverage   40.54%   40.76%   +0.22%     
==========================================
  Files         406      407       +1     
  Lines       54501    54735     +234     
==========================================
+ Hits        22095    22311     +216     
+ Misses      29370    29369       -1     
- Partials     3036     3055      +19
Impacted Files Coverage Δ
routers/user/setting/applications.go 45.61% <0%> (-2.47%) ⬇️
models/repo_list.go 66.84% <0%> (-1.06%) ⬇️
routers/user/setting/oauth2.go 0% <0%> (ø) ⬆️
routers/repo/editor.go 28.67% <0%> (ø) ⬆️
routers/api/v1/repo/git_hook.go 78.7% <0%> (ø)
routers/routes/routes.go 81.99% <0%> (+0.02%) ⬆️
modules/mailer/mailer.go 22.51% <0%> (+0.05%) ⬆️
routers/api/v1/convert/convert.go 81.31% <0%> (+0.63%) ⬆️
routers/api/v1/api.go 73.06% <0%> (+0.66%) ⬆️
modules/setting/mailer.go 50.7% <0%> (+0.7%) ⬆️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8d01b98...2e38f3e. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 16, 2019
@@ -6,41 +6,20 @@ if [ ! -d /data/git/.ssh ]; then
fi

if [ ! -f /data/git/.ssh/environment ]; then
echo "GITEA_CUSTOM=/data/gitea" >| /data/git/.ssh/environment
echo "GITEA_CUSTOM=${GITEA_CUSTOM}" >| /data/git/.ssh/environment
Copy link
Contributor

Choose a reason for hiding this comment

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

A change of GITEA_CUSTOM will not propagate to the /data/git/.ssh/environment file.

See PR #6608 for a workaround:
https://github.com/go-gitea/gitea/pull/6608/files#diff-e9383835b6feff19b2e8f36745b70414R12

Copy link
Member Author

Choose a reason for hiding this comment

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

We should wait for your PR to get merged and I will rebased this PR on.

#!/bin/bash

# Define the environment variables
export DATA_PATH=${DATA_PATH:-"/data/gitea"}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are these hidden in a service file?

Except for the INSTALL_LOCK they can all go into the Dockerfile.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes I didn't want to cluter the dockerfile but still wanted to have a dedicated file detailing all the env variable possible to use and their default. But it could be in the Dockerfile.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added a comment to the Dockerfile to indicate to go see this file.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that the various config options seem to clutter the Dockerfile.

Removing the GITEA_CUSTOM environment variable breaks the ad-hoc commands.

# docker run -d --name gitea_no_env gitea:sapk
# docker exec -it gitea_no_env gitea dump
2019/04/17 13:14:08 ...s/setting/setting.go:501:NewContext() [W] Custom config '/usr/local/bin/custom/conf/app.ini' not found, ignore this if you're running first time
<snip>

Copy link
Member

Choose a reason for hiding this comment

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

Why DATA_PATH? APP_DATA_PATH is already a setting in app.ini we should keep name same for same purpose.

@kolaente
Copy link
Member

Please resolve conflicts.

@stale
Copy link

stale bot commented Jul 12, 2019

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions.

@stale stale bot added the issue/stale label Jul 12, 2019
@stale
Copy link

stale bot commented Sep 10, 2019

This pull request has been automatically closed because of inactivity. You can re-open it if needed.

@stale stale bot closed this Sep 10, 2019
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/stale lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants