-
Notifications
You must be signed in to change notification settings - Fork 18
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
lib: use random values for ranges, closes #632 #647
Conversation
def _generate_random_range_cut(start: int = 0, end: int = 10): | ||
start = random.randint(start, end) | ||
end = random.randint(start, end) | ||
return f"{start}:{end}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if it's 10:10 or 0:0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assumed these are valid ranges, at least the command works:
COMMAND: neofs-cli --config /home/runner/work/neofs-node/neofs-node/neofs-testcases/wallet_config.yml object range --rpc-endpoint 's01.neofs.devenv:8080' --wallet '/home/runner/work/neofs-node/neofs-node/neofs-testcases/TemporaryDir/9565822e-6a8a-4dd6-8191-d1e79649b0bd.json' --cid '61M7pv5kkku8yNhrLiohmLd31XuSDCR3WLQkiGrd4Sgd' --oid 'B5vP6r1BQ3ktxY2wfvDNwBy6UnKYTRAQRCCqcCzUgGsb' --range '10:10' --file 'TemporaryDir/TestObjectsDir/3bbbe60d-6fdd-4b04-9449-0fd2e8826521'
RETCODE: 0
STDOUT:
[TemporaryDir/TestObjectsDir/3bbbe60d-6fdd-4b04-9449-0fd2e8826521] Payload successfully saved
STDERR:
Start / End / Elapsed 19:31:43.302994 / 19:31:43.707277 / 0:00:00.404283
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed according to Pavel's comment
def _generate_random_range_cut(start: int = 0, end: int = 10): | ||
start = random.randint(start, end) | ||
end = random.randint(start, end) | ||
return f"{start}:{end}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that is not [start:end]
meaning but is [offset:height]
in fact. so i would like to ask you to rename it accordingly to prevent any misunderstanding. also, we do not allow [X:0]
requests, do we have any tests for it? just to fix it
btw:
▶ neofs-cli object range -h | grep '\-\-range'
--range string Range to take data from in the form offset:length
so we are not that bad and try to doc it (i can imagine we would have written it as just "Range to take data from"). but i agree that slicing format ([start:end]
) may be more presentative
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for additional tests - #652
fixed naming and the code accordingly
320a6eb
to
d36cd4a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK otherwise.
Signed-off-by: Evgeniy Zayats <[email protected]>
d36cd4a
to
9be6b6b
Compare
No description provided.