Skip to content

Conversation

@joasode
Copy link
Contributor

@joasode joasode commented Nov 29, 2024

A variety of changes are attempted in this restructure. The primary goal was to create a separation between the SingularitySandbox and CondaInstall classes which were tightly coupled. The following are done to accomplish this:

  • Make SingularitySandbox only responsible for the opening, building and closing the sandbox folder and required environment files.
  • Only expose the required sandbox directory and environment filename to CondaInstall. (This will also make testing easier as we just need to provide two strings instead of mocking the complex sandbox object)
  • Move most communication methods away from SingularitySandbox to a separate communication interface (currently just a few util functions, but could be streamlined further).

Additionally, I noticed a few anti-patterns that are fixed to some extend.

  • The verbosity settings could be defined in various different places, which is changed to having a single-source-of-truth
  • The option to have log_dispatcher=None seem to be purely for tests and not used in production. Additionally, as it is sometimes None, I have made the log_settings a required variable so log_dispatcher can always handle the logging.
    -- Note, It becomes quite confusing which object decides what should be logged, such as _display_message in CondaInstall which does one thing in tests, but possibly another thing in production as far as I can see. (This should probably be absorbed completely into the log_dispatcher... )
  • Particularly for CondaInstall; Instead of initializing big classes with many internal variables and running BigClass.method() with either no input or output or both, I give meaningful inputs and outputs to the methods, which should make it more clear what is required- and the expected result- of the methods.

A bunch of tests are disabled, although almost only tests with hardcoded monkeypatch behavior. (And the 80 unit width CLI tests, which break when pytest is executed in emacs terminal).

The core functionality CLI commands currently works, including building a container with a conda environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants