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

rippling cli: Added flux build init command #7

Merged
merged 6 commits into from
Apr 24, 2024
Merged

Conversation

vguptarippling
Copy link
Contributor

@vguptarippling vguptarippling requested review from a team as code owners April 22, 2024 19:06
Copy link

@sz2376 sz2376 left a comment

Choose a reason for hiding this comment

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

Approved with comments

.gitignore Show resolved Hide resolved
rippling_cli/utils/login_utils.py Outdated Show resolved Hide resolved
rippling_cli/utils/file_utils.py Outdated Show resolved Hide resolved
# TODO: Since run configuration cannot be transferred from import/export settings , it lies inside .idea folder in \
# the project directory. This should be a separate command \
# https://intellij-support.jetbrains.com/hc/en-us/community/posts/206600965-Export-Import-Run-Configurations
def create_run_configurations(project_name: str):
Copy link

Choose a reason for hiding this comment

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

where is this function called?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought of initially including this part of command but looks like for pycharm run/debug configuration cannot be transferred so we will have to create .idea folder and copy the xml file inside the folder. I am planning to add this as part of run_server command as I am hoping that app-developer would have opened the project in pycharm. Since adding this file before in this command creates problem with pycharm settings which they pycharm does on project open

rippling_cli/cli/commands/flux/build.py Show resolved Hide resolved
rippling_cli/core/setup_project.py Outdated Show resolved Hide resolved


def install_pip():
subprocess.run([sys.executable, "-m", "ensurepip", "--default-pip"])
Copy link

Choose a reason for hiding this comment

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

[Security]. Detected subprocess function 'run' without a static string.

Developer action:

If this data can be controlled by a malicious actor, it may be an instance of command injection.
Also, don't use shell=True wherever possible. Audit the use of this call to ensure it is not controllable by an external resource.

💬 Reply with /semgrep ignore <reason> or triage in Semgrep Cloud Platform to ignore the finding created by dangerous-subprocess-use.

from typing import Optional

import click
import requests # type: ignore
Copy link

Choose a reason for hiding this comment

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

[best-practice]. Untyped 'type: ignore' or specific typed 'type: ignore' comments like 'attr-defined', 'method-assign', 'name-defined', 'operator' should be avoided as they can hide real issues.

💬 Reply with /semgrep ignore <reason> or triage in Semgrep Cloud Platform to ignore the finding created by development_avoid_type_ignore_error.

@@ -12,3 +13,4 @@ def flux(ctx: click.Context) -> None:


flux.add_command(app) # type: ignore
flux.add_command(build) # type: ignore
Copy link

Choose a reason for hiding this comment

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

[best-practice]. Untyped 'type: ignore' or specific typed 'type: ignore' comments like 'attr-defined', 'method-assign', 'name-defined', 'operator' should be avoided as they can hide real issues.

💬 Reply with /semgrep ignore <reason> or triage in Semgrep Cloud Platform to ignore the finding created by development_avoid_type_ignore_error.

@vguptarippling vguptarippling merged commit 4732bd6 into main Apr 24, 2024
4 checks passed
@vguptarippling vguptarippling deleted the APPS-25574 branch May 6, 2024 08:50
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.

2 participants