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

[Fix] --log-outputs relative path are not supported when --log-rotate-config-json is defined #13049

Merged
merged 2 commits into from
Jun 8, 2021

Conversation

mumuhhh
Copy link
Contributor

@mumuhhh mumuhhh commented May 27, 2021

--log-outputs relative path are not supported when --log-rotate-config-json is defined
for example: --log-rotate-config-json --log-outputs log/test.log

u, err := url.Parse("rotate:log/test.log")
u.Path is ""

例如  rotate:test.log 路径解析不正确
@ptabor
Copy link
Contributor

ptabor commented May 27, 2021

Please translate to English.

@mumuhhh mumuhhh changed the title [Fix]滚动日志路径解析不正确 [Fix] --log-outputs relative path are not supported when --log-rotate-config-json is defined May 27, 2021
@ptabor ptabor requested a review from hexfusion May 27, 2021 11:30
@hexfusion hexfusion self-assigned this May 27, 2021
Copy link
Contributor

@hexfusion hexfusion left a comment

Choose a reason for hiding this comment

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

nice catch!

server/embed/config_logging.go Show resolved Hide resolved
TestLogRotation add test log output relative path test
@ptabor ptabor requested a review from hexfusion June 2, 2021 16:06
Copy link
Contributor

@lilic lilic left a comment

Choose a reason for hiding this comment

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

We should backport this fix to 3.5 as well?

@gyuho gyuho merged commit 940d1e1 into etcd-io:main Jun 8, 2021
@gyuho
Copy link
Contributor

gyuho commented Jun 8, 2021

@hexfusion can we backport this to 3.5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants