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

Fix #5111: Trim whitespace from custom images #5112

Merged
merged 2 commits into from
Aug 7, 2020
Merged

Fix #5111: Trim whitespace from custom images #5112

merged 2 commits into from
Aug 7, 2020

Conversation

ianhellstrom
Copy link
Contributor

@ianhellstrom ianhellstrom commented Jul 7, 2020

See: #5111

It trims the whitespace around custom images to avoid pod creation failures. It does so in the backend and Rok template, where customImage is explicitly set.


Note: I tried running both the frontend and backend on my own machine but could not make it work. Perhaps the instructions for npm and Flask are outdated. I ran Flask in a fresh virtual environment.

NPM:

../src/create_string.cpp:17:25: error: no matching constructor for initialization of 'v8::String::Utf8Value'
  v8::String::Utf8Value string(value);
                        ^      ~~~~~

Flask:

2020-07-07 13:43:47,944 | kubeflow_jupyter.default.app | INFO | Sending file 'index.html'
127.0.0.1 - - [07/Jul/2020 13:43:47] "GET / HTTP/1.1" 500 -
Traceback (most recent call last):
  File "/Users/ian/Python/backend/lib/python3.7/site-packages/flask/app.py", line 1813, in full_dispatch_request
    rv = self.dispatch_request()
  File "/Users/ian/Python/backend/lib/python3.7/site-packages/flask/app.py", line 1799, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "/Users/ian/Code/oss/kubeflow/components/jupyter-web-app/backend/kubeflow_jupyter/default/app.py", line 77, in serve_root
    return send_from_directory("./static/", "index.html")
  File "/Users/ian/Python/backend/lib/python3.7/site-packages/flask/helpers.py", line 709, in send_from_directory
    raise NotFound()
werkzeug.exceptions.NotFound: 404 Not Found: The requested URL was not found on the server. If you entered the URL manually please check your spelling and try again.

@googlebot
Copy link

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@kubeflow-bot
Copy link
Contributor

This change is Reviewable

@k8s-ci-robot
Copy link
Contributor

Hi @ianhellstrom. Thanks for your PR.

I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot requested review from prodonjs and vkoukis July 7, 2020 11:33
@ianhellstrom ianhellstrom changed the title Trim whitespace from custom images Fix #5111: Trim whitespace from custom images Jul 7, 2020
@ianhellstrom
Copy link
Contributor Author

@googlebot I consent.

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: yes and removed cla: no labels Jul 7, 2020
@ianhellstrom
Copy link
Contributor Author

/assign @kimwnasptd

@prodonjs
Copy link
Contributor

/uncc @prodonjs

@k8s-ci-robot k8s-ci-robot removed the request for review from prodonjs July 29, 2020 11:01
@@ -47,7 +47,7 @@ export class RokJupyterLabSelectorComponent implements OnInit {
}

setLabValues(lab: JupyterLab) {
this.parentForm.get("customImage").setValue(lab.image);
this.parentForm.get("customImage").setValue(lab.image.trim());
Copy link
Member

Choose a reason for hiding this comment

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

I think you meant to use the .strip() function here right?

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 don't think so, but I may be wrong.

Copy link
Member

Choose a reason for hiding this comment

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

@ianhellstrom my bad, I got confused for a second and thought this was also python code

@kimwnasptd
Copy link
Member

@ianhellstrom the changes look good, thanks for the PR!

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kimwnasptd

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 845af29 into kubeflow:master Aug 7, 2020
saffaalvi pushed a commit to StatCan/kubeflow that referenced this pull request Feb 11, 2021
* Trim whitespace for custom image names

* Trim whitespace from custom image

Co-authored-by: Ian Hellström <ihellstrom@d2iq.com>
saffaalvi pushed a commit to StatCan/kubeflow that referenced this pull request Feb 12, 2021
* Trim whitespace for custom image names

* Trim whitespace from custom image

Co-authored-by: Ian Hellström <ihellstrom@d2iq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants