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

Docker run restart policy support #40

Draft
wants to merge 8 commits into
base: development
Choose a base branch
from

Conversation

omesser
Copy link
Contributor

@omesser omesser commented Jan 6, 2021

  • Restart policy flags can now be defined as an Image property
  • Minor changes/fixes in tests
  • Added pycharm runConfigurations
  • Added some non default properties to test shell commands

@omesser omesser closed this Dec 9, 2021
@omesser omesser reopened this Dec 9, 2021
Copy link
Collaborator

@liranbg liranbg left a comment

Choose a reason for hiding this comment

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

Thanks for finalizing this PR
minor comment to make it merge-ready

@@ -53,7 +53,7 @@ def main():
# (from a local dir if it exists)
# "python -m pip install" instead of "pip install" handles a pip
# issue where it fails in a long-named dir
run('virtualenv --python=python3 venv && '
run('virtualenv --python=python3.7 venv && '
Copy link
Collaborator

Choose a reason for hiding this comment

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

since we also support manof atop python 3.9, this needs to stay python3 and below, on project.toml, leave it to be either 3.7 or 3.9

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@liranbg - thanks for the review!
Getting back to this now. I understand manof needs compatibility with both 3.7 and 3.9, but since this is an install script, several questions:

  1. developers - I suppose they should all align to 3.9, right? there should be a single developer version for the repo. even if it's overridable, or BC with other python version. traditionally this is the highest supported version. Should we change CI scripts to install and test 3.9?
  2. Is there a scenario where install script is used sometimes in 3.7.X env and sometimes in 3.9.x env? I think it's only used in the makefile for dev environment. but maybe in the internal iguazio deployment script it is called from somewhere as well so that's why it should support both?

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.

None yet

2 participants