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

Code for create application #71

Merged
merged 9 commits into from
Apr 11, 2023
Merged

Code for create application #71

merged 9 commits into from
Apr 11, 2023

Conversation

yawjalik
Copy link
Contributor

@yawjalik yawjalik commented Mar 8, 2023

Implementing the command deploifai application create NAME [FLAGS]

Users can specify these using flags:

  • Container image name using -i --image
  • Container image tag using -t or --tag
  • Port -p or --port

If not provided, users will be prompted along with other details:

  • cloud profile
  • app plan depending on provider of cloud profile
  • environment variables

@yawjalik
Copy link
Contributor Author

yawjalik commented Mar 8, 2023

@utkarsh867 this is what I got so far

@utkarsh867
Copy link
Member

Great! Let me get through quickly.

@utkarsh867
Copy link
Member

@yawjalik There are two scenarios here for the Docker image that we should handle.

  1. The user has a Docker registry and the image url is of that registry: In this case, the Docker image will have a https:// in the URL. Hence we use this image URL as it is.
  2. Image is a local image: It will be a local image. We use our servers to create the registry for the user in the push process. We can have an output in the terminal to tell the user that we created the registry.

All that happening automatic, without user prompt will be insane.

The only catch to this I see is that Dockerhub images don't need a URL. Not sure how we can handle that. @98sean98 You have any ideas how to resolve this?

@yawjalik
Copy link
Contributor Author

yawjalik commented Mar 15, 2023

Updates:

  • Option for local image or publicly accessible image
  • Option to create new repository or use existing repository (deploifai-managed)
  • Tag and push image to the selected repository uri

TODO:

  • Error handling
  • Handle external repositories
  • Check for invalid tags (the pushing process doesn’t throw an error)
  • Potentially need to check for repository name restrictions (e.g. AWS has specific requirements for the repository name)

@98sean98 98sean98 added the enhancement New feature or request label Mar 16, 2023
@98sean98
Copy link
Contributor

98sean98 commented Mar 16, 2023

Updates:

  • Option for local image or publicly accessible image
  • Option to create new repository or use existing repository (deploifai-managed)
  • Tag and push image to the selected repository uri

TODO:

  • Error handling
  • Handle external repositories
  • Check for invalid tags (the pushing process doesn’t throw an error)
  • Potentially need to check for repository name restrictions (e.g. AWS has specific requirements for the repository name)

Pretty good work.

  1. try to catch and handle the exceptions that you think can most likely happen
  2. we can build the feature to push to an existing (non-deploifai managed) repository in the future, or maybe we don't
  3. do u mean invalid local image tags? need more elaboration
  4. our backend api actually demands that the container registry name to be
  • between 4 and 30 characters
  • must be lowercase alphanumeric and not contain spaces or special characters other than -

in my experience, these are the requirements that aws, azure, and gcp sort of demand
see backend code

@yawjalik
Copy link
Contributor Author

  1. Accept multiple -e or --env flags either with one or more KEY=VALUE values or single env filepath path/to/file
  2. Call get_application to poll for deployment status every 10 seconds
  3. Added click spinner for prettier loading

click.secho(f"\nApplication {application['name']} created successfully", fg="green")

# Poll for deployment status every 10 seconds
click.echo("Checking deployment status (this will take some time)... ", nl=False)
Copy link
Contributor

Choose a reason for hiding this comment

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

"Deploying application (this will take a few minutes, and it's safe to CTRL+C) ..."

# Format env
formatted_env = []
for env_var in env:
key, val = env_var.split("=", maxsplit=1)
Copy link
Contributor

Choose a reason for hiding this comment

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

Catch python ValueError when env_var fails to unpack into a key-value pair, this would indicate that this env_var is faulty.

Print this env_var for the user, and abort the command

Comment on lines 15 to 19
@click.option(
"--env",
help="Environment variables for the container (e.g. --env path/to/env OR --env VAR1=VAL1 --env VAR2=VAL2)",
multiple=True,
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Add another --env-file option to read env files. This should be able to accept multiple files as well.

If the user uses both --env and --env-file options, just read everything and aggregate into one list.

{
"type": "list",
"name": "config",
"message": "Choose a size for the application",
Copy link
Contributor

Choose a reason for hiding this comment

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

Choose a size of the application ({provider})

@yawjalik
Copy link
Contributor Author

yawjalik commented Apr 8, 2023

@98sean98 removed config stuff and should be ready for merge? Any other issues?

@98sean98 98sean98 merged commit ea4ade1 into main Apr 11, 2023
@98sean98 98sean98 deleted the create-app branch April 11, 2023 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants