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

Send error logs through the event API with the correct task/subtask #6516

Merged
merged 3 commits into from
Aug 26, 2021

Conversation

MarlonGamez
Copy link
Contributor

Fixes: https://github.com/GoogleCloudPlatform/cloud-code-vscode-internal/issues/5281

Description
This PR ensures that error messages are getting sent with the correct task and subtask information through the Event API.

@MarlonGamez MarlonGamez requested a review from a team as a code owner August 25, 2021 23:23
@MarlonGamez MarlonGamez requested a review from tejal29 August 25, 2021 23:23
@google-cla google-cla bot added the cla: yes label Aug 25, 2021
@MarlonGamez MarlonGamez changed the title Send error logs through the event API in the correct task/subtask Send error logs through the event API with the correct task/subtask Aug 25, 2021
@tejal29 tejal29 added this to the v1.29.0 milestone Aug 26, 2021
@tejal29
Copy link
Contributor

tejal29 commented Aug 26, 2021

hmm so different than #6515 :)
However, I am ok with this approach in case you feel more confident about this.

@codecov
Copy link

codecov bot commented Aug 26, 2021

Codecov Report

Merging #6516 (0324458) into main (997edc8) will increase coverage by 0.00%.
The diff coverage is 75.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #6516   +/-   ##
=======================================
  Coverage   70.43%   70.43%           
=======================================
  Files         515      515           
  Lines       23127    23142   +15     
=======================================
+ Hits        16289    16301   +12     
- Misses       5780     5783    +3     
  Partials     1058     1058           
Impacted Files Coverage Δ
cmd/skaffold/skaffold.go 0.00% <0.00%> (ø)
pkg/skaffold/event/v2/event.go 84.58% <83.33%> (-0.06%) ⬇️
pkg/skaffold/event/v2/build.go 100.00% <100.00%> (ø)
pkg/skaffold/event/v2/deploy.go 100.00% <100.00%> (ø)
pkg/skaffold/util/tar.go 63.21% <0.00%> (-2.30%) ⬇️
pkg/skaffold/docker/parse.go 88.23% <0.00%> (+0.84%) ⬆️

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 d3839f4...0324458. Read the comment docs.

MarlonGamez and others added 2 commits August 26, 2021 08:32
@MarlonGamez
Copy link
Contributor Author

MarlonGamez commented Aug 26, 2021

Thanks for the review @tejal29 ! My reasoning for the different approach was that I felt it was simpler to handle this in TaskFailed and the SubtaskFailed funcs as we wouldn't have to go and add print statements to the various failure points of build, deploy, etc, but rather just use the function that should already be called in any of those cases.

@tejal29 tejal29 merged commit a46ffd5 into GoogleContainerTools:main Aug 26, 2021
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.

2 participants