-
Notifications
You must be signed in to change notification settings - Fork 9
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
TG 839 #38
TG 839 #38
Conversation
Co-authored-by: rboni-dk <[email protected]>
dk-installer.py
Outdated
os.makedirs(data_folder, exist_ok=True) | ||
else: | ||
script_path = pathlib.Path(sys.argv[0]).absolute() | ||
data_folder = script_path.parent.joinpath("DataKitchenApps") |
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.
This will change the folder name on Linux, etc. Is this expected?
dk-installer.py
Outdated
if args.skip_verify: | ||
return | ||
|
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.
Sorry, I don't remember if I've asked this before, but why are we removing the --skip-verify option?
dk-installer.py
Outdated
if detect_os() == 'Windows': | ||
self.docker_compose_file = get_docker_compose_path() | ||
else: | ||
self.docker_compose_file = pathlib.Path() / DOCKER_COMPOSE_FILE |
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.
This is confusing. Isn't the get_docker_compose_path()
handling this?
|
||
|
||
class TestGenSetupDatabaseStep(Step): | ||
label = "Initializing the platform database" | ||
required = False |
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.
@FernandezAstor , why are we changing this to False
?
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.
It was part of the oldest PR:
https://github.com/DataKitchen/data-observability-installer/pull/35/files
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.
Oh, I see.. Wait.. you were adding the timeout to the network removal, right? What happened to that?
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.
yes, but Aarthy told me to leave it as it was originally.
No description provided.