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

Add non zero exit code status for errors in dss #78

Closed
misohu opened this issue Apr 17, 2024 · 2 comments · Fixed by #90
Closed

Add non zero exit code status for errors in dss #78

misohu opened this issue Apr 17, 2024 · 2 comments · Fixed by #90
Labels
enhancement New feature or request

Comments

@misohu
Copy link
Member

misohu commented Apr 17, 2024

Why it needs to get done

It is a good practice to add non zero exit codes when the CLI fails. These are some recommendations.

What needs to get done

Add exit codes for errors in DSS

When is the task considered done

DSS returns with exit code on failure.

@misohu misohu added the enhancement New feature or request label Apr 17, 2024
Copy link

Thank you for reporting us your feedback!

The internal ticket has been created: https://warthogs.atlassian.net/browse/KF-5578.

This message was autogenerated

@misohu
Copy link
Member Author

misohu commented Apr 22, 2024

Sync notes:
Every module function and utils function is only catching expected exceptions e.g. lightkube.ApiErrors and is reacting to those. Each expected error in module name should

  • use debug to store the error with traceback
  • use logger.error to print descriptive error message according to spec
  • use logger.info to print hints according to spec
  • raise an empty RuntimeError()

e.g.

try:
        lightkube_client.replace(obj)
        logger.info(
            f"Stopping the notebook {name}. Check `dss list` for the status of the notebook."
        )
        return
except ApiError as e:
        logger.error(f"Failed to stop Notebook {name}.")
        logger.debug(f"Failed to scale down Deployment {name} with error: {e}.", exc_info=True)
        raise RuntimeError()

Every command in main should:

  • handle RuntimeError without logging anything else, just by exiting with status code 1 (loggin was done in the module function)
  • handle any other Exception by providing descriptive error message and use debug to store error and traceback and exit with status code 1.

e.g.

@main.command(name="stop")
@click.option(
    "--kubeconfig",
    help=f"Path to a Kubernetes config file. Defaults to the value of the KUBECONFIG environment variable, else to '{KUBECONFIG_DEFAULT}'.",  # noqa E501
)
@click.argument("notebook_name", required=True)
def stop_notebook_command(kubeconfig: str, notebook_name: str):
    """
    Stops a running notebook in the DSS environment.
    \b
    Example:
        dss stop my-notebook
    """
    try:
        kubeconfig = get_default_kubeconfig(kubeconfig)
        lightkube_client = get_lightkube_client(kubeconfig)
        stop_notebook(name=notebook_name, lightkube_client=lightkube_client)
    except RuntimeError:
        click.get_current_context().exit(1)
    except Exception as e:
        logger.debug(f"Failed to stop notebook: {e}.", exc_info=True)
        logger.error(f"Failed to stop notebook: {str(e)}.")
        click.get_current_context().exit(1)

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 a pull request may close this issue.

1 participant