Conversation
End-rey
requested review from
carpawell,
cthulhu-rider and
roman-khimov
as code owners
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #3790 +/- ##
==========================================
- Coverage 25.52% 25.51% -0.02%
==========================================
Files 660 663 +3
Lines 42636 42673 +37
==========================================
+ Hits 10884 10889 +5
- Misses 30749 30783 +34
+ Partials 1003 1001 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
End-rey
force-pushed
the
policy-probe-util
branch
from
9bbeb3f to
7b9ea0b
Compare
| ) | ||
|
|
||
| var testPolicyCmd = &cobra.Command{ | ||
| Use: "test-policy", |
There was a problem hiding this comment.
This can also be policy to me. Maybe policy try like we have acl basic print, although here it's application to a particular network that matters. I'm open to other suggestions, but I'd try to avoid test as well.
There was a problem hiding this comment.
Are you suggesting a separate policy command with a single try command?
There was a problem hiding this comment.
Yes. Maybe some other policy things could be done in future. But I'd like to hear @carpawell and @cthulhu-rider as well.
There was a problem hiding this comment.
i can imagine serverless container policy parse --policy 'EC 3/1 CBF 2 SELECT ...'. Taking this into account, container policy try seems better
at the same time, try seems too abstract to me. I suggest container policy apply
There was a problem hiding this comment.
if it is possible not to use - in a command name and there is subcomand relation (policy is a common thing, try is one of the things that can be done with policies), i would vote for policy try
There was a problem hiding this comment.
I'm in favor of the policy try/apply/check command.
@roman-khimov Which one should we choose?
End-rey
force-pushed
the
policy-probe-util
branch
2 times, most recently
from
2b79a01 to
b430c89
Compare
Add a CLI utility to validate container policies and display nodes that are policy-compliant in the current epoch. Closes #3626. Signed-off-by: Andrey Butusov <andrey@nspcc.io>
Signed-off-by: Andrey Butusov <andrey@nspcc.io>
| Long: `Check placement policy parsing and validation. | ||
| Policy can be provided as QL-encoded string, JSON-encoded string or path to file with it. | ||
| Shows nodes that will be used for container placement based on current network map snapshot.`, | ||
| Args: cobra.NoArgs, |
There was a problem hiding this comment.
policy can naturally be an arg (not a flag) imo, but do not insist
| return fmt.Errorf("unable to get netmap snapshot to validate container placement: %w", err) | ||
| } | ||
|
|
||
| placementNodes, err := nm.ContainerNodes(*placementPolicy, cid.ID{}) |
There was a problem hiding this comment.
@cthulhu-rider, is it possible to get a different result in some corner cases if we intentionally put cid.ID instead of a real container id?
| return &result, nil | ||
| } | ||
|
|
||
| return nil, errors.New("can't parse placement policy") |
There was a problem hiding this comment.
with no details at all? can be hard to debug for a user, even in --verbose mode
There was a problem hiding this comment.
oh, that is a copy from another file, ok
4 participants
Closes #3626.