Fix shardmanager at upload

storj-archived / storj-python-sdk

Deprecated. A Python SDK for the Storj API.

https://storj.io

MIT License

23 stars 19 forks source link

Fix shardmanager at upload #125

Closed marcorosa closed 7 years ago

marcorosa commented 7 years ago

I propose the following changes in class ShardManager:

let the user define the number of shards and not the dimension of the shards (I think it is more "user friendly")
if the user defines the number of shards (parameter num_chunks), use it! In fact, at the current version, we let the user specify the shard dimension but this parameter is not taken into consideration.
Function get_optimal_shard_parameters returns the number of shards and their dimension, but the dimension is not used. So I propose to change it into get_optimal_shard_number since what we use is only the number of shards

steenzout commented 7 years ago

I just have a couple of questions:

If you change the number of shards that you'll use will that interfere with previous file downloads? from my little knowledge I'm assuming it's independent.
will this make the Python client behave even more differently that the official client?

marcorosa commented 7 years ago

This change doesn't affect file download since the ShardManager class (used for upload) is independent from the ShardingTool class (used for download). A further step can be merging these 2 classes into one.

I've taken a look into the storjlib before this PR, and there seems to be no possibility for the user to decide the size or the number of shards.

steenzout commented 7 years ago

@marcorosa that's what I thought.

I'll still merge this PR after the build passes.

coveralls commented 7 years ago

Coverage remained the same at 46.827% when pulling 0217146adf01e5a036a7e01d71b4d72c6cb9701d on marcorosa:fix-sharder into 86d60932c508dd1f32d830eabce39226be9c9976 on Storj:develop.

coveralls commented 7 years ago

Coverage remained the same at 46.827% when pulling 0217146adf01e5a036a7e01d71b4d72c6cb9701d on marcorosa:fix-sharder into 86d60932c508dd1f32d830eabce39226be9c9976 on Storj:develop.

marcorosa commented 7 years ago

@steenzout test fixed according to previous commits