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

URLs in Artifact viewer do not display underscores right "_" #3424

Closed
paveldournov opened this issue Apr 2, 2020 · 11 comments
Closed

URLs in Artifact viewer do not display underscores right "_" #3424

paveldournov opened this issue Apr 2, 2020 · 11 comments
Assignees
Labels
area/frontend area/pipelines priority/p1 status/triaged Whether the issue has been explicitly triaged

Comments

@paveldournov
Copy link
Contributor

In the artifact pane the URLs shown for Metadata artifacts do not display underscores and instead turn the text to italic. When the user copies the URL - it gets copied without the underscores and thus does not point to a valid file.

image

image

@numerology
Copy link

/assign @numerology

@numerology
Copy link

The culprit is that underscore is not properly escaped when TFX image writing the markdown string

See [1]

[1] https://github.com/tensorflow/tfx/blob/master/tfx/orchestration/kubeflow/container_entrypoint.py#L218

@Bobgy
Copy link
Contributor

Bobgy commented Apr 2, 2020

There may be other formatting conflicts later, I think instead of escaping, we should wrap data with ``

@rmgogogo
Copy link
Contributor

rmgogogo commented Apr 3, 2020

if quick fix, we may put it to OSS 1.0

@Bobgy Bobgy added area/frontend priority/p1 status/triaged Whether the issue has been explicitly triaged and removed area/front-end labels Apr 3, 2020
@Bobgy
Copy link
Contributor

Bobgy commented Apr 3, 2020

I'm leaning avoid escaping underscore in UI, adding hack on top of a proper markdown parser is fragile.

e.g. how do we not escape underscores that are already escaped? how many other corner cases there are?

@Bobgy
Copy link
Contributor

Bobgy commented Apr 3, 2020

I think the best fix is to render execution detail page as a tab and let tfx side remove the markdown. Execution detail page has all the information in the best accessible form.

@numerology
Copy link

I think the best fix is to render execution detail page as a tab and let tfx side remove the markdown. Execution detail page has all the information in the best accessible form.

Sounds good. Anyway I'm quickly fix it by escaping underscore in the entrypoint.

@Bobgy
Copy link
Contributor

Bobgy commented Apr 20, 2020

@numerology did you fix the issue? With #3457 we may also remove the markdown. (but we can keep it for a while before people can upgrade KFP installation).

@numerology
Copy link

@numerology did you fix the issue? With #3457 we may also remove the markdown. (but we can keep it for a while before people can upgrade KFP installation).

It should be fixed in tensorflow/tfx@3ea79f9

@Bobgy
Copy link
Contributor

Bobgy commented Apr 21, 2020

@numerology thanks!
let's close this then
/close

@k8s-ci-robot
Copy link
Contributor

@Bobgy: Closing this issue.

In response to this:

@numerology thanks!
let's close this then
/close

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/frontend area/pipelines priority/p1 status/triaged Whether the issue has been explicitly triaged
Projects
None yet
Development

No branches or pull requests

6 participants