-
Notifications
You must be signed in to change notification settings - Fork 1
docker create and rm logic added #1
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
Conversation
@@ -1,5 +1,6 @@ | |||
class Configuration: | |||
path = "/Users/olegsokolov/PycharmProjects/piper/applications" | |||
path = "/home/pavel/repo/piper_new_out/" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А можешь просто динамически находить как-то папку
типо
piper_path = os.getcwd()
applications_path = f"{piper_path}/applications
или вообще их может в /tmp/ сохранять, хотя лучше пока явно где-то рядом с основным
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
я бы вообще это в setting.py вынес, потому что юзеру решать где он хочет свой проект развернуть. А если свойство не указано, тогда на уровень выше и как текущая папка + '_out'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А в чем отличие setting.py от configurations.py будет, более глобальные настройки?
import docker | ||
import time | ||
import sys | ||
from loguru import logger |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Прикольная вещь
piper/utils/docker_utils.py
Outdated
stop_result = stop_container(docker_client, cont_id) | ||
logger.info('stoped', stop_result) | ||
status = 'exited' | ||
time.sleep(3) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Зачем тут sleep? вообще их нехорошо конечно нам везде вставлять, лучше опираться на сигналы от докера различные
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
да, тут лишние, тут можно опросить докер
по поводу sleep в container.run то средствами докера это не решить, докер говорит что статус изменен
с created на running, но внутри приложение еще не запустилось. мне кажется логично в fast api добавить
health_check и когда получим ответ, продолжать.
Отлично 🙌 |
docker create and rm logic added
добавлена логика проверки существования контейнера и образа.