Make quantity field human readable

[matdexir]

Description

Allows for human readable quantity field in the different json config, i.e, 10KB -> 10000. This implementation still allows for the field being pure unsigned integers, so, it can be seen as offering an alternative configuration

Fixes #862

Type of change

Please delete options that aren't relevant.

• [ ] Bug fix (non-breaking change which fixes an issue)
• [x] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
• [ ] This change requires a documentation update

How Has This Been Tested?

I tested it on a stripped version of basic_cas.json which allows for writing human readable units rather than large integer values.

Checklist

• [ ] Updated documentation if needed
• [x] Tests added/amended
• [x] bazel test //... passes locally
• [x] PR is contained in a single commit, using git amend see some docs

---

This change is 

[matdexir]

Hi @allada, Is there a specific test suite for this feature? Can't seem to get bazel test ... to run properly.

[matdexir]

I am currently trying to get bazel test ... to run but for some reasons it fails. Here are the relevant logs:

ERROR: /home/matdexir/Files/Projects/nativelink/nativelink-config/BUILD.bazel:9:13: Rustfmt nativelink-config/nativelink-config.rustfmt.ok failed: (Exit 1): process_wrapper failed: error executing Rustfmt command (from target //nativelink-config:nativelink-config) bazel-out/k8-opt-exec-ST-13d3ddad9198/bin/external/rules_rust~0.42.1/util/process_wrapper/process_wrapper --touch-file bazel-out/k8-fastbuild/bin/nativelink-config/nativelink-config.rustfmt.ok -- ... (remaining 11 arguments skipped)
Diff in /tmp/nativelink/work/508fecf4260be788cf6e32983d75215d677579d6b11cb2bc2d23db421ee8cfb8/work/nativelink-config/src/serde_utils.rs at line 12:
 // See the License for the specific language governing permissions and
 // limitations under the License.

-use byte_unit::Byte;
-use humantime::parse_duration;
 use std::fmt;
 use std::marker::PhantomData;
 use std::str::FromStr;
Diff in /tmp/nativelink/work/508fecf4260be788cf6e32983d75215d677579d6b11cb2bc2d23db421ee8cfb8/work/nativelink-config/src/serde_utils.rs at line 20:

+use byte_unit::Byte;
+use humantime::parse_duration;
 use serde::{de, Deserialize, Deserializer};

I am not too sure what this means since it looks fine to my human eye. Maybe I forgot to update some settings?

[matdexir]

The most convenient solution I came up with was:

fn visit_str<E: de::Error>(self, v: &str) -> Result<Self::Value, E> {
    let expanded = (*shellexpand::env(v).map_err(de::Error::custom)?).to_string();
    let byte_size = Byte::parse_str(expanded, true).map_err(de::Error::custom)?;
    let byte_size_u128 = byte_size.as_u128();
    T::try_from(byte_size_u128.try_into().map_err(de::Error::custom)?)
       .map_err(de::Error::custom)
}

What do you think?

Also for the duration section, casting it seconds yields a u64 value rather than u128. I was wondering if this would be ideal (Although I could use as u128)? Or maybe I could cast it into {milli, micro, nano}(they all yield u128) seconds and convert it into seconds accordingly?

[matdexir]

I'm currently missing a test case for duration values. I haven't found any config files that could make use of duration units from here. Any idea on where I could get one?

[matdexir]

That's odd :thinking: It works all fine on my local machine. Could you give me a reproducible build with docker?

FYI. I am using nix OS for dev. Here are my specs: OS: Arch Linux Kernel: 6.8.7-arch1-1

[CLAassistant]

All committers have signed the CLA.

[MarcusSorealheis]

@matdexir do you need a hand further here?

[matdexir]

I think that it should be fixed now. I also have updated it to match the latest release. Let me know if the issue still lingers

NOTE: Currently nativelink-store tests are failing but I am not too sure why since it should technically be out of scope for my current PR

cc: @allada @aaronmondal @MarcusSorealheis

[MarcusSorealheis]

looks like you need to rebase @matdexir

[matdexir]

Already did It should be up to date with https://github.com/TraceMachina/nativelink/commit/0a33c8399e38e9aeb1d76c41f0663d16e9f938ec