Store_map and some improvements

[Synicix]

Updated (03/06/2024)

From the top to bottom of files change
Cargo.toml:

• Change excessive nesting threshold due to 317 in docker.rs due to input blob checking
• add module_name_repetitions for renaming Store to ModelStore

crypto.rs

• Add a wrapper around std::io::error to include path (Helps with debugging problems both on development and later usage, standard std::io error is kind of useless in the case where you don't know what path is causing the problem)

error.rs

• Added a bunch of errors including the io::error wrapper mention in the previous line

model.rs

• Update code to reflect the changes made by the hashing pull request
• Change pod_job input_stream_path and output_stream_path to map to more accurately reflect the new behavior
• Add retry policy that was missing, but there is no tests for it yet (Will create and issue)
• Simplify blob to just to be file and folder and made output_stream_map use a new struct Output (this is mainly because output doesn't need checksum and the kind should always be folder)
• Add new and resolve_absolute_path to blob where new will compute checksum upon creation, and resolve_absolute_path will give caller the full path back given a store_map
• Rename FileOrFolder to PathType to be more general
• Add Output struct that is use for pod_job output which is similar to Blob except with kind and checksum removed
• Add retry policy struct (Technically this could be a separate pull request.

docker.rs

• Remove data_directory from the struct definition as that is no longer needed in the new design with store_map
• Add store map as a requirement for all functions dealing with pod_job
• Update start_with_altimage to provide better error for std::io::error (Was annoying to debug because it was io::error, but wasn't telling what it couldn't access)
• Update list containers to handle the case where there are multiple containers that matches (I don't recall exactly what was the problem, but there was an issue a few weeks ago where failed containers (python code failure) was not cleared off the daemon and got listed [Need to create an issue to investigate this futher)
• Update get_result to obtain logs from container (Was debugging failed container, and this feature was super useful and needed to be added)
• Update prepare_container_start_inputs to check for actual existence of inputs before continuing (Had an issue where it would fail because it the file didn't exist, and it mounted it as an empty directory anyways. This was a silent failure and was fairly annoying to debug)

mod.rs

• Update to support store_map

filestore.rs

• Move statics to the top (That is just my preference, can change it back)
• Updated lookup hash to handle the case where multiple hashes were found for some reason
• Remove DEFAULT_DATA_NAMESPACE since that is no longer needed

mod.rs

• Add store_map_fixture to reflect the new store_map design
• Remove Fake store and and changed store_fixture

model.rs

• Update model.rs tests to reflect changes above

orchestrator.rs

• Update tests to support store_map

store.rs

• Update tests to support store_map

[Synicix]

Updated the pull request with the new redesign of the store pointer and hashing system. Please ignore the orchestrator failing tests. There are some cases that are not being caught, causing it to fail and currerntly working on fixing it.

[guzman-raphael]

We didn't get to discuss in our meeting today so I'll drop a summary here from my review.

• Retry
  • Didn't realize until finishing the review that it isn't implemented (only placeholders in model)
  • In that case, we should separate this into a separate PR/issue for a complete feature
• Logs
  • Didn't realize this was in until reviewing
  • The feature is missing key elements from the issue (get_logs_blocking(), get_logs())
  • Given that, I think this would be best suited for it's own PR
• IoError wrapper(s) or similar
  • Spent quite a bit of time looking into this
  • Unfortunately, there isn't much low-level options from fs
  • thiserror also isn't exposing ways to add context
  • snafu seems like an interesting option that actually seems to be recommended by several articles
  • Worried about our error lib growing to a level that would be challenging to maintain (common complaint of thiserror)
  • Perhaps rather than putting energy into manually adding it, we could instead pull this into a separate PR for converting us to snafu (if it makes sense)
• Without the above, the PR boils down to
  • Removing the default user file data e.g. from store, orchestrator
  • Removing BlobInterface
  • Removing util::hash
  • Simplifying design of PodJob spec
  • Adding a namespace lookup i.e. namespace -> local path
  • Hash Folder and Collection inputs
  • Handle binds for Collection inputs
• After reviewing, I made this branch with some ideas.

We can probably go over in more detail when we tagup next.

[guzman-raphael]

Nice collab @Synicix! 💪