Open
Conversation
Collaborator
|
Hey @timmjd, Thank you so much for the great work you put in to make this change. I will set aside some time this week or next week to go through these changes, but from the glimpse of it, it seems like a very useful addition to the hook! |
nikola-jokic
requested changes
May 31, 2024
Collaborator
nikola-jokic
left a comment
There was a problem hiding this comment.
Can you please fix types in tests. They can be fixed with simple as Type. Other than that, this looks good to me
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As a self hosted runner administrator i want so know why my runners are crashing. If this is related to k8s, a stack trace often does help to pin down the origin of the issue.
Therefore, this change does improve the debug capability of the k8s hook implementation by providing a stack trace via the debug output if an exception happend.
Before the change
After the change
Change summary
The primary change is to enable a correct
sourceMapoutput for the code, so the stack trace will show up the.tsorigin with the correct line number of the trace and not some location within the minified / bakedindex.js.packages/hookliband thepackages/k8sandpackages/dockerwas changed from a JS based include into a TypeScript composite build. To make this happen, thebaseUrl&rootUrlrequired adaptation.tscfollowed bynccwas replaced with a direct call ofnccdue to otherwise the line number reference got lost.k8s/package.jsonthat appear now due to the linkage is happening now on a typed based linkage. I guess there was also a bug within themainContainerContextPorts[port.containerPort]assignment.nccandtypescriptlibrary update was required due to the used version caused a bug.