This repository has been archived by the owner on Oct 31, 2023. It is now read-only.
Epic: Computational environments refactoring #4129
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Used `golem.envs` instead of `golem.environments` because we don't want to replace the old environments *yet*. The old package will be removed when we are done. Environment events should probably be somehow defined. We can think of using `NewType` for all ID types so that one would have to use the type explicitly.
It's slightly different than restart_ctx() and makes more sense to use when reconfiguring the VM: * restart_ctx -> force VM restart * reconfig_ctx -> put machine in appropriate state for reconfiguration but don't power it up if it wasn't running before the call Also provided default implementations for all the contexts in Hypervisor abstract class.
Subtyping NamedTuple caused some weird typechecker issues.
+ Added hypervisor error handling in prepare() and cleanup()
Creating _Data classes was necessary because NamedTuple must be the only superclass for its direct subclasses.
* Use existing `DictSerializable` instead of a new class * Make `Hypervisor.is_available()` an abstract method * Copy data in `from_dict()` methods not to modify arguments * Rename `prepare_prerequisites()` to `install_prerequisites()`
And two minor fixes
Having arguments as a separate field only complicates things. They can be added as part of `command` string.
Non Docker-specific methods for managing status changes have been moved to the Runtime abstract base class.
Also fixed a minor bug with warnings format (Docker API returns `None` when there are no warnings).
client.attach_socket() returns different types depending on OS. This caused issues with stdin on Linux.
Calling shutdown() on a closed socket would raise an error so a flag was added to make sure that this doesn't happen. Added a comment, removed trailing newline.
Whitelist the image to be downloaded. Check if download was successful.
Docker input & output
Starting status update loop before setting status to 'RUNNING' would make the loop exit instantly and not update status.
ENV_ENABLED -> ENABLED ENV_DISABLED -> DISABLED
Callback could potentially execute for a long time so defer them not to block the main thread.
Environment events & runtime events
We don't need this. All commands are executed via docker-py.
Because 'cleanup' is a noun and method names should be verbs.
Environment refactoring cleanup
mfranciszkiewicz
approved these changes
May 24, 2019
Codecov Report
@@ Coverage Diff @@
## develop #4129 +/- ##
===========================================
+ Coverage 88.46% 88.73% +0.26%
===========================================
Files 216 222 +6
Lines 18806 19574 +768
===========================================
+ Hits 16637 17369 +732
- Misses 2169 2205 +36 |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.