-
Notifications
You must be signed in to change notification settings - Fork 167
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
Prevent download of empty files in Pipeline Logs #2229
Prevent download of empty files in Pipeline Logs #2229
Conversation
...end/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/runLogs/DownloadDropdown.tsx
Show resolved
Hide resolved
frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/runLogs/LogsTab.tsx
Show resolved
Hide resolved
frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/runLogs/LogsTab.tsx
Outdated
Show resolved
Hide resolved
da8abfd
to
2512659
Compare
frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/runLogs/LogsTab.tsx
Outdated
Show resolved
Hide resolved
2512659
to
fa03a33
Compare
frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/runLogs/LogsTab.tsx
Outdated
Show resolved
Hide resolved
fa03a33
to
bf2688c
Compare
frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/runLogs/LogsTab.tsx
Outdated
Show resolved
Hide resolved
2d7713a
to
26570ac
Compare
frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/runLogs/LogsTab.tsx
Outdated
Show resolved
Hide resolved
26570ac
to
5d88a9c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
5d88a9c
to
476d3d0
Compare
@DaoDaoNoCode Oh yes, I missed this. Thanks for pointing this out. Added now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/runLogs/LogsTab.tsx
Outdated
Show resolved
Hide resolved
476d3d0
to
60998ed
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code lgtm, needs to run through it tomorrow to test. My internet is still down now for the whole afternoon so I got very limited internet access.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested, works fine.
/lgtm
60998ed
to
617082d
Compare
...end/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/runLogs/DownloadDropdown.tsx
Outdated
Show resolved
Hide resolved
...end/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/runLogs/DownloadDropdown.tsx
Outdated
Show resolved
Hide resolved
...end/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/runLogs/DownloadDropdown.tsx
Outdated
Show resolved
Hide resolved
...end/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/runLogs/DownloadDropdown.tsx
Outdated
Show resolved
Hide resolved
frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/runLogs/LogsTab.tsx
Outdated
Show resolved
Hide resolved
frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/runLogs/LogsTab.tsx
Outdated
Show resolved
Hide resolved
frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/runLogs/LogsTabStatus.tsx
Outdated
Show resolved
Hide resolved
...end/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/runLogs/DownloadDropdown.tsx
Outdated
Show resolved
Hide resolved
frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/runLogs/LogsTab.tsx
Outdated
Show resolved
Hide resolved
frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/runLogs/LogsTabStatus.tsx
Outdated
Show resolved
Hide resolved
617082d
to
0a503ef
Compare
frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/runLogs/LogsTab.tsx
Outdated
Show resolved
Hide resolved
frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/runLogs/LogsTabStatus.tsx
Outdated
Show resolved
Hide resolved
0a503ef
to
a4a797e
Compare
@@ -121,6 +115,7 @@ const LogsTabForPodName: React.FC<{ podName: string }> = ({ podName }) => { | |||
<LogsTabStatus | |||
loaded={loaded} | |||
error={error} | |||
isLogsAvailable={podContainers.length !== 1 && logs !== ''} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel the logic is not correct here... if you simply do !(podContainers.length === 1 && !logs)
you will get podContainers.length !== 1 || !!logs
a4a797e
to
8ed1301
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: DaoDaoNoCode 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 |
The request changes have been addressed
ffa7afd
into
opendatahub-io:f/pipelines-enhancement
JIRA: RHOAIENG-259
Description
This PR :
GIF for raw logs:
Screen.Recording.2023-11-30.at.7.35.07.PM.mov
Screenshots for view raw logs and disabled and enabled states of both Raw logs and Download button:
Enabled state:
Disabled:
Small screen:
How Has This Been Tested?
Test Impact
Added tests to check whether the drawer opens on clicking the node of a pipeline run
Request review criteria:
Self checklist (all need to be checked):
If you have UI changes:
After the PR is posted & before it merges:
main