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

feat: show an error if the docker image identifier is invalid #3369

Open
wants to merge 1 commit into
base: andrea/jupyter-free-build
Choose a base branch
from

Conversation

andre-code
Copy link
Contributor

@andre-code andre-code commented Oct 18, 2024

PR to show an error if the docker image identifier is invalid.

Screenshot 2024-10-18 at 17 07 37

Screenshot 2024-10-18 at 17 08 31

Notion ref: https://www.notion.so/renku/Show-an-error-if-the-docker-image-identifier-is-invalid-1190df2efafc809682caee2d8c485285

/deploy renku=feat-jupyter-free-sessions renku-data-services=release-amaltheas-migration amalthea-sessions=main renku-notebooks=master renku-gateway=1.1.0 extra-values=amalthea-sessions.deployCrd=false

@andre-code andre-code requested a review from a team as a code owner October 18, 2024 09:21
@RenkuBot
Copy link
Contributor

You can access the deployment of this PR at https://renku-ci-ui-3369.dev.renku.ch

Base automatically changed from andrea/use-data-service-sessions-2.0 to andrea/jupyter-free-build October 18, 2024 14:03
@andre-code andre-code force-pushed the andrea/validate-image-when-launcing-session branch from d4de56a to 4fe7acc Compare October 18, 2024 14:50
@andre-code andre-code changed the title [wip] feat: show an error if the docker image identifier is invalid feat: show an error if the docker image identifier is invalid Oct 18, 2024
Copy link
Member

@leafty leafty left a comment

Choose a reason for hiding this comment

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

Code looks good. I would like to check the deployment before approving.

projectUrl: string;
}) {
if (!("status" in error)) {
return false;
Copy link
Member

Choose a reason for hiding this comment

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

You should not return false for a component.

Suggested change
return false;
return null;

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.

3 participants