Mtor/prod 15381 #443
Mtor/prod 15381 #443
Conversation
MohcineTor
force-pushed
the
mtor/PROD-15381
branch
from
5f6f434 to
6ec31a6
Compare
| ) | ||
|
|
||
|
|
||
| def dict_to_tfvars(payload: dict) -> str: |
There was a problem hiding this comment.
I would suggest to break this in smaller steps. First expose the terraform to the end user to avoid hard bugs, when the codebase is more mature we can then hide it and make things modular.
| tf_dir = Path(str(env.working_dir)).parent / "terraform-webapp" | ||
| tfvars_path = tf_dir / "terraform.tfvars" | ||
|
|
||
| if "win" in current_os: |
There was a problem hiding this comment.
win could be cygwin and I don't think we want this. From the docs https://docs.python.org/3/library/sys.html#sys.platform the comparisson should probably be something like:
if current_os == 'win32':
[...]
elif current_os == 'linux':
[...]
else:
raise error
| return None | ||
| project_yaml_files = ["Organization.yaml", "Solution.yaml", "Workspace.yaml", "Dataset.yaml", "Runner.yaml"] | ||
| tf_webapp_path = Path(getcwd()) / "terraform-webapp" | ||
| repo_url = "https://github.com/Cosmo-Tech/terraform-webapp.git" |
There was a problem hiding this comment.
We are introducing some hidden dependencies here. Now Babylon depends on terraform-webapp which depends on terraform etc and this is not visible in the Babylon metadata (in pyprojects.toml). This will make tools that automatically scan dependencies (e.g. dependency track) to have a blind spot, and also users installing Babylon will not know they need to configure this dependency.
I suggest we first decide if this is an optional or required dependency: it would be optional if we have a use case where Babylon user would not need this dependency.
Then we should update the readme.md to include this and possibly also add a check to make sure the tools are available.
Finally we would need to somehow add the external dependency in pyproject.toml. There is a pending PEP to include external dependencies, but since it has not yet been accepted we need to find a way to do it that is compatible with the build toolchain (PEP 725 & 804).
| logger.error(f" [bold red]✘[/bold red] Terraform directory not found at {tf_dir}") | ||
| return | ||
| try: | ||
| process = subprocess.Popen( |
There was a problem hiding this comment.
why not the simpler subprocess.run ? do we need an advanced behavior here?
There was a problem hiding this comment.
there are way too many levels of if/else here, sonarqube will flag this as too complex
| logger.error(f" [bold red]✘[/bold red] Terraform directory not found at {tf_dir}") | ||
| return | ||
| try: | ||
| process = subprocess.Popen( |
There was a problem hiding this comment.
there are way too many levels of if/else here, sonarqube will flag this as too complex
2 participants
Hi,
new feat deploy web app with babylon using terraform module.