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

Watch taskrun and open debug if taskrun is running with debug mode #636

Merged
merged 4 commits into from
Nov 3, 2021

Conversation

sudhirverma
Copy link
Contributor

@sudhirverma sudhirverma commented Oct 19, 2021

Fix #637

@sudhirverma sudhirverma changed the title Open debug if any taskrun is running with debug mode Watch taskrun and open debug if taskrun is running with debug mode Oct 19, 2021
@codecov-commenter
Copy link

codecov-commenter commented Oct 20, 2021

Codecov Report

Merging #636 (6877514) into master (37114de) will increase coverage by 0.03%.
The diff coverage is 70.64%.

❗ Current head 6877514 differs from pull request most recent head dfbe4bb. Consider uploading reports for the commit dfbe4bb to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #636      +/-   ##
==========================================
+ Coverage   68.33%   68.36%   +0.03%     
==========================================
  Files         111      114       +3     
  Lines        6483     6534      +51     
  Branches     1138     1149      +11     
==========================================
+ Hits         4430     4467      +37     
- Misses       2053     2067      +14     
Impacted Files Coverage Δ
src/debugger/debugExplorer.ts 46.66% <ø> (ø)
src/extension.ts 77.72% <31.25%> (-3.23%) ⬇️
src/debugger/delete-debugger.ts 50.00% <50.00%> (ø)
src/tekton/restartpipelinerundata.ts 39.06% <50.00%> (+0.72%) ⬆️
src/debugger/debug-tree-view.ts 58.97% <57.14%> (+5.64%) ⬆️
src/tekton/pipelinerun.ts 82.00% <57.14%> (+0.36%) ⬆️
src/util/watchResources.ts 73.61% <82.35%> (+0.72%) ⬆️
src/tekton/task-run-data.ts 83.33% <83.33%> (ø)
src/debugger/cancel-taskrun.ts 100.00% <100.00%> (ø)
src/debugger/debug-continue.ts 100.00% <100.00%> (ø)
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 37114de...dfbe4bb. Read the comment docs.

@sudhirverma sudhirverma marked this pull request as ready for review October 24, 2021 03:31
@@ -65,7 +71,8 @@ async function startTaskRunWithDebugger(taskRun: TektonNode, commandId?: string)
generateName: `${taskRun.getName()}-`
}
}
const taskRunContent = await TaskRun.getTaskRunData(taskRun.getName());
const taskRunContent = await getTaskRunData(taskRun.getName());
if (!taskRunContent) return null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

it would be better if you indicate that this function can return null

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 have added the return type for taskRunContent

@@ -19,6 +19,7 @@ export class PipelineRun extends TektonItem {
static async restart(pipelineRun: TektonNode, commandId?: string): Promise<void> {
if (!pipelineRun) return null;
const trigger = await pipelineRunData(pipelineRun);
if (!trigger) return null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

this function already return void, so returning null is ambiguous

import { tkn } from '../tkn';


export async function getTaskRunData(taskRunName: string): Promise<TknTaskRun>{
Copy link
Collaborator

Choose a reason for hiding this comment

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

this not an utility function, this file should be in another folder

@@ -89,6 +94,15 @@ export class WatchResources {
treeRefresh.set('treeRefresh', false);
pipelineExplorer.refresh();
}
} else if (run.kind === 'TaskRun' && run.spec?.['debug'] && semver.satisfies('0.26.0', `<=${getNewELSupport.pipeline}`) && run.status?.conditions[0]?.status === 'Unknown') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

'0.26.0' should be constant

Copy link
Collaborator

@evidolob evidolob left a comment

Choose a reason for hiding this comment

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

All good, except first review comment

@sudhirverma sudhirverma merged commit ecea5d5 into redhat-developer:master Nov 3, 2021
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.

Watch taskrun and open debug if taskrun is running with debug mode
3 participants