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

Let users save logs #1040

Merged
merged 1 commit into from
Dec 15, 2016
Merged

Let users save logs #1040

merged 1 commit into from
Dec 15, 2016

Conversation

spadgett
Copy link
Member

@spadgett spadgett commented Dec 14, 2016

Adds a "Save" link to the log viewer that saves the log contents as a file. Closes #356

https://trello.com/c/OaF7o3U3

Saves just the log text for logs that have links or colors. Does not add in the line numbers.

openshift_web_console

Uses library https://github.com/eligrey/FileSaver.js which is very small (< 200 lines of code). See browser support here:

https://github.com/eligrey/FileSaver.js#supported-browsers

Tested with Chrome, Firefox, Safari, IE, and iOS. On iOS, it opens a new window with the plain log text:

screen shot 2016-12-14 at 2 22 54 pm

cc @benjaminapetersen

@jwforres
Copy link
Member

I really hesitate to call this Save if it's only copying the part of the log that we have loaded

@spadgett
Copy link
Member Author

We could warn if it's only part of the log (and give the CLI command)

@benjaminapetersen
Copy link
Contributor

Nice to get this feature back with such a small lib!

@spadgett
Copy link
Member Author

Something like this:

screen shot 2016-12-14 at 4 30 03 pm

@jwforres
Copy link
Member

jwforres commented Dec 15, 2016 via email

@spadgett
Copy link
Member Author

Sure but the command should be the full width copy to clipboard

Yeah, needs a custom dialog to use the directive. Working on it.

@spadgett
Copy link
Member Author

@jwforres Updated. Note I added some tests for CLIHelp (bottom of diff).

Here is the current dialog:

screen shot 2016-12-15 at 8 59 06 am

@jwforres
Copy link
Member

jwforres commented Dec 15, 2016 via email

@spadgett
Copy link
Member Author

does the command line tools link open in a new tab?

Yes

@jwforres
Copy link
Member

What happens if we have 5000 really long log lines and we hit the 500MiB limit in some of the browsers? Or does the log viewer already fall over at that point :)

@jwforres
Copy link
Member

Can you add a comment above that line that if we ever increase that limit that we can't go over the Blob limit

@jwforres
Copy link
Member

otherwise this LGTM

Copy link
Member

@jwforres jwforres left a comment

Choose a reason for hiding this comment

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

approve pending adding that comment

@spadgett
Copy link
Member Author

Thanks @jwforres, added the comment about limitBytes.

[merge]

Adds a "Save" link to the log viewer that saves the log contents as
a file.
@spadgett
Copy link
Member Author

Fixed typo in test string

[merge]

@spadgett
Copy link
Member Author

[merge]

@openshift-bot
Copy link

Evaluated for origin web console merge up to 6ba8911

@openshift-bot
Copy link

openshift-bot commented Dec 15, 2016

Origin Web Console Merge Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_web_console/861/) (Base Commit: 8e40552)

@openshift-bot openshift-bot merged commit 247492a into openshift:master Dec 15, 2016
@spadgett spadgett deleted the save-log branch December 16, 2016 17:24
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.

4 participants