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/Do not print false warning about deprecated config #1654

Merged
merged 1 commit into from
Aug 4, 2022
Merged

Fix/Do not print false warning about deprecated config #1654

merged 1 commit into from
Aug 4, 2022

Conversation

carpawell
Copy link
Member

@carpawell carpawell commented Aug 3, 2022

No description provided.

@carpawell carpawell added the bug Something isn't working label Aug 3, 2022
@carpawell carpawell self-assigned this Aug 3, 2022
@carpawell carpawell marked this pull request as ready for review August 3, 2022 17:54
@codecov
Copy link

codecov bot commented Aug 3, 2022

Codecov Report

Merging #1654 (b0ee381) into master (4558f30) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

❗ Current head b0ee381 differs from pull request most recent head 01a413f. Consider uploading reports for the commit 01a413f to get more accurate results

@@            Coverage Diff             @@
##           master    #1654      +/-   ##
==========================================
- Coverage   33.18%   33.17%   -0.02%     
==========================================
  Files         332      332              
  Lines       22706    22714       +8     
==========================================
  Hits         7536     7536              
- Misses      14552    14560       +8     
  Partials      618      618              
Impacted Files Coverage Δ
pkg/local_object_storage/engine/tree.go 0.00% <0.00%> (ø)
pkg/services/object/acl/acl.go 30.23% <0.00%> (ø)

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

}

if timeout == 0 {
timeout = time.Second * defaultTimeout
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not make defaultTimeout * time.Second the constant itself?

Comment on lines 40 to 42
cfg.SetDefault("profiler.address", "localhost:6060")
cfg.SetDefault("profiler.shutdown_timeout", "30s")

cfg.SetDefault("metrics.address", "localhost:9090")
Copy link
Contributor

Choose a reason for hiding this comment

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

This used the old values because of the need to support both sections.
I think we can remove already deprecated sections in the next release and simplify the code considerably.

…tions

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
@carpawell
Copy link
Member Author

@fyrchik, changed the idea of the PR: just dropped the deprecated naming.

@carpawell carpawell added this to the v0.31.0 milestone Aug 4, 2022
@carpawell carpawell merged commit 0a60524 into nspcc-dev:master Aug 4, 2022
@carpawell carpawell deleted the fix/printing-deprecated-message branch August 4, 2022 12:38
@alexchetaev alexchetaev added the U3 Regular label Aug 23, 2022
aprasolova pushed a commit to aprasolova/neofs-node that referenced this pull request Oct 19, 2022
…onfig sections

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working U3 Regular
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants