neofs-cli: bugfix expire-at flag for session create command. #2546
Conversation
AliceInHunterland
requested review from
roman-khimov,
carpawell and
cthulhu-rider
as code owners
Codecov Report
@@ Coverage Diff @@
## master #2546 +/- ##
=======================================
Coverage 29.66% 29.67%
=======================================
Files 405 405
Lines 30756 30756
=======================================
+ Hits 9123 9126 +3
+ Misses 20863 20861 -2
+ Partials 770 769 -1 see 2 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
AliceInHunterland
force-pushed
the
bugfix/2539-expire-at-flage-check
branch
from
b393929
to
c3c6d50
Compare
|
|
||
| exp, _ := cmd.Flags().GetUint64(commonflags.ExpireAt) | ||
| lifetime := uint64(defaultLifetime) | ||
| if lfArg, _ := cmd.Flags().GetUint64(commonflags.Lifetime); lfArg != 0 { | ||
| exp = currEpoch + lfArg | ||
| if cmd.Flags().Changed(commonflags.Lifetime) { |
There was a problem hiding this comment.
IINW may be shortened to
var exp uint64
if cmd.Flags().Changed(commonflags.ExpireAt) {
// Cobra guarantees that commonflags.Lifetime is unset then (see init)
exp, _ = cmd.Flags().GetUint64(commonflags.ExpireAt)
if exp <= currEpoch {
common.ExitOnErr(cmd, "", errors.New("expiration epoch must be greater than current epoch"))
}
} else {
lifetime, _ := cmd.Flags().GetUint64(commonflags.Lifetime)
exp = currEpoch + lifetime
}
var tok session.ObjectThere was a problem hiding this comment.
I think we also need a check of passing --lifetime 0
There was a problem hiding this comment.
suggest to add separate check for this
There was a problem hiding this comment.
if exp <= currEpoch {
common.ExitOnErr(cmd, "", errors.New("expiration epoch must be greater than current epoch"))
}
this should be enough, no?
There was a problem hiding this comment.
technically enough, but smth like lifetime must not be zero would be more clearer, right?
i'm ok to leave if exp <= currEpoch check below as before cuz it doesn't directly related to the bug. But if u find such clarification worthy - add new commit
CHANGELOG.md
Outdated
| @@ -39,6 +39,7 @@ minor release, the component will be purged, so be prepared (see `Updating` sect | |||
| - `neofs-lens write-cache list` command duplication (#2505) | |||
| - `neofs-adm` works with contract wallet in `init` and `update-contracts` commands only (#2134) | |||
| - Missing removed but locked objects in `SEARCH`'s results (#2526) | |||
| - `--exire-at` flag in `neofs-cli session create` command (#2539) | |||
There was a problem hiding this comment.
| - `--exire-at` flag in `neofs-cli session create` command (#2539) | |
| - `--expire-at` flag in `neofs-cli session create` command (#2539) |
btw was it broken in the latest release? if not, then this record is redundant
AliceInHunterland
force-pushed
the
bugfix/2539-expire-at-flage-check
branch
2 times, most recently
from
8314f6c
to
2eea1e7
Compare
AliceInHunterland
force-pushed
the
bugfix/2539-expire-at-flage-check
branch
from
2eea1e7
to
3343cd0
Compare
Added a check for changes to the lifetime flag when it's passed by the user to prevent the settlement of the default lifetime value even if expire-at flag passed. Closes #2539. Signed-off-by: Ekaterina Pavlova <[email protected]>
AliceInHunterland
force-pushed
the
bugfix/2539-expire-at-flage-check
branch
from
3343cd0
to
a43f240
Compare
Added check of passing
--lifetimeflag by the user and handling of settingdefaultLifetimevalueCloses #2539.