Revert "Disallow duplicate storage paths (#26484)" #29171
Conversation
This reverts commit 92fda9c.
GiteaBot
added
the
lgtm/need 2
pull-request-size
bot
added
the
size/M
wxiaoguang
added
the
skip-changelog
GiteaBot
added
lgtm/need 1
|
|
||
| func fatalDuplicatedPath(name, p string) { | ||
| if targetName, ok := uniquePaths[p]; ok && targetName != name { | ||
| log.Fatal("storage path %q is being used by %q and %q and all storage paths must be unique to prevent data loss.", p, targetName, name) |
There was a problem hiding this comment.
Wouldn't it suffice to convert the Fatal to an Error for now?
I know, it doesn't fix the problem completely, but it would be an improvement already…
There was a problem hiding this comment.
Then it comes to the same question:
- How many users really care about the startup logs .... I have answered many questions about "why something doesn't work" but the startup log already told users.
- What an user could do if they see such message. Any document or guideline?
There was a problem hiding this comment.
Let me throw the ball right back at you:
Yes, it is in no way a complete solution, I've understood that already.
However, what is better?
- Keeping a fix that while not perfect does work at scaring (some) people from shooting themselves in the foot
- Disabling this check completely because we don't have any idea what recovery measures are available
That's exactly the problem: Everyone who has this problem has to decide how to proceed best themselves.
Gitea does not know which file belongs where. That's the original problem we want to fix.
I don't see any other way than letting each affected admin decide what should be moved where.
I'm in favor of at least keeping the log.Error.
Originally, before you notified me that all Docker users have this problem by default, I would have even argued for Fatal, but that's a valid problem that we have caused ourselves.
As such, we cannot Fatal anymore, but we can at least make every user (that reads the logs) aware of the problem.
And in the meantime, we can work on a better fix.
|
I do not agree with this revert, if there are invalid or missing checks they should be fixed but gitea should still fail to start with invalid configuration that could lead to data loss |
What's your suggestion to address these concerns?
|
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
Let me re-explain my opinion here: I proposed to revert that change, not because of its purpose, but because of various problems of it:
Anyway, I don't have enough time at the moment. So I will close this revert. Leave the documentation / wrong code to people have time to fix (I might have time in the future, but not now ....) |
|
Automatically locked because of our CONTRIBUTING guidelines |
|
I think this revert is still necessary before the v1.22 stable release. It may prevent many instances from starting. We can revert this one and rework the PR because fixing it is very complicated. If no obvious objection, I will merge it in a week. |
|
I made a new one: Refactor startup path check #30169 |
Revert #26484
Discussion: #26484 (comment) and #26484 (comment)