Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Executor improvements Part 01 #1924

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

Conversation

mpvgithub
Copy link
Contributor

Changed

  • Modified the executor core to change the file path for terraform state files to store and read from .config/executor_plugins folder
  • Clean up any half-done/dirty deployed resources post deploy up

@mpvgithub mpvgithub self-assigned this Feb 1, 2024
@mpvgithub mpvgithub requested a review from a team as a code owner February 1, 2024 10:18
@mpvgithub mpvgithub changed the title Executor improvements Executor improvements Part 01 Feb 1, 2024
from covalent.executor import _executor_manager

logger = logging.getLogger()
logger.setLevel(logging.ERROR)
handler = logging.StreamHandler(sys.stderr)
logger.addHandler(handler)
logger.propagate = False
sdk_constants = get_default_sdk_config()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use get_config instead of directly importing the defaults since they might've been changed by the user in the config file directly. Using get_config with specifically what is needed (instead of the entire sdk config) will take that into consideration as well.

@@ -389,7 +397,24 @@ def _get_tf_statefile_path(self) -> str:

"""
# Saving in a directory which doesn't get deleted on purge
return str(Path(self.executor_tf_path) / "terraform.tfstate")
return str(Path(self.executor_tfstate_path) / "terraform.tfstate")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Saving in a directory which doesn't get deleted on purge

self.executor_tfstate_path seems to point to ~/.config/covalent/executor_plugins/*, this WILL get deleted by purge. We need to add an exception in the purge command to prevent deletion of these state files.

Comment on lines +413 to +416
# Saving in a directory which doesn't get deleted on purge
state_path = os.path.join(parent_path, self.executor_name)
if not os.path.exists(state_path):
os.makedirs(state_path)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as above. One minor thing is that if possible let's try to use pathlib's Path instead of os.path to join/checking existence/checking whether its a directory/etc.

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