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

Feat/Support changing network config in adm #1593

Merged
merged 1 commit into from
Jul 12, 2022
Merged

Feat/Support changing network config in adm #1593

merged 1 commit into from
Jul 12, 2022

Conversation

carpawell
Copy link
Member

No description provided.

@carpawell carpawell added enhancement Improving existing functionality neofs-adm NeoFS Adm application issues labels Jul 8, 2022
@carpawell carpawell self-assigned this Jul 8, 2022
@codecov
Copy link

codecov bot commented Jul 8, 2022

Codecov Report

Merging #1593 (90b2d41) into master (fdf62e8) will decrease coverage by 0.09%.
The diff coverage is 6.55%.

❗ Current head 90b2d41 differs from pull request most recent head 81f59d1. Consider uploading reports for the commit 81f59d1 to get more accurate results

@@            Coverage Diff             @@
##           master    #1593      +/-   ##
==========================================
- Coverage   35.65%   35.56%   -0.10%     
==========================================
  Files         305      305              
  Lines       18212    18273      +61     
==========================================
+ Hits         6494     6498       +4     
- Misses      11168    11225      +57     
  Partials      550      550              
Impacted Files Coverage Δ
cmd/neofs-adm/internal/modules/morph/config.go 0.00% <0.00%> (ø)
pkg/services/object/acl/acl.go 30.23% <0.00%> (-0.72%) ⬇️
cmd/neofs-adm/internal/modules/morph/root.go 50.00% <50.00%> (ø)

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 3a2c025...81f59d1. Read the comment docs.

@carpawell carpawell marked this pull request as ready for review July 8, 2022 12:55
Comment on lines 288 to 289
// print some warning that user
// want to add some unknown config?
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add --force flag to this command? Then fail here if it is missing.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

sure, added

@carpawell carpawell requested a review from fyrchik July 12, 2022 10:33
fyrchik
fyrchik previously approved these changes Jul 12, 2022
return fmt.Errorf("can't get netmap contract hash: %w", err)
}

forceFlag, err := cmd.Flags().GetBool(forceConfigSet)
Copy link
Contributor

Choose a reason for hiding this comment

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

Just ignore the error?

Copy link
Member Author

Choose a reason for hiding this comment

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

hm, yes, we do ignore that kind of error usually. dropped that check

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
@fyrchik fyrchik merged commit 466b9b4 into nspcc-dev:master Jul 12, 2022
@carpawell carpawell deleted the feat/support-updating-config-via-adm branch July 12, 2022 18:09
aprasolova pushed a commit to aprasolova/neofs-node that referenced this pull request Oct 19, 2022
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
enhancement Improving existing functionality neofs-adm NeoFS Adm application issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants