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.

Sign up for GitHub

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

Jump to bottom

Enhanced error logging #5698

Merged
merged 5 commits into from
May 22, 2024
Merged

Enhanced error logging #5698

merged 5 commits into from
May 22, 2024

Conversation

NeerajNagure
Copy link
Contributor

@NeerajNagure NeerajNagure commented May 16, 2024 •
edited

Description of your changes

Added template name to errApplyComposed so that we can get more info about the resource

Fixes #5488

I have:

Need help with this checklist? See the cheat sheet.

@NeerajNagure NeerajNagure requested review from negz and a team as code owners May 16, 2024 05:45
@NeerajNagure NeerajNagure requested a review from bobh66 May 16, 2024 05:45
negz
negz reviewed May 16, 2024
View reviewed changes
Copy link
Member

@negz negz left a comment

Choose a reason for hiding this comment

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

Thank you!

internal/controller/apiextensions/composite/composition_pt.go Outdated Show resolved Hide resolved
@NeerajNagure NeerajNagure requested a review from negz May 17, 2024 04:51
@negz
Copy link
Member

negz commented May 17, 2024 •
edited

@NeerajNagure Apart from renaming to errFmt... this looks good to me. Can you please fill out the checklist on the PR description per the linked cheat sheet.

Edit: Also, the unit tests are failing. Please follow the instructions in the checklist - run make reviewable to catch this kind of thing earlier.

@NeerajNagure NeerajNagure requested a review from negz May 18, 2024 07:14
@NeerajNagure
Copy link
Contributor Author

@negz I have made all the necessary changes and fixed unit tests

negz
negz approved these changes May 20, 2024
View reviewed changes
Copy link
Member

@negz negz left a comment

Choose a reason for hiding this comment

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

Looks good, thank you!

@negz
Copy link
Member

negz commented May 20, 2024

Hrm, actually it looks like make check-diff is failing. Can you rebase on master and run make reviewable again please.

@NeerajNagure NeerajNagure requested a review from a team as a code owner May 21, 2024 04:44
@NeerajNagure
Copy link
Contributor Author

@negz I have merged the latest changes from master branch and also ran make reviewable

@NeerajNagure NeerajNagure requested a review from negz May 21, 2024 04:59
@negz
Copy link
Member

negz commented May 21, 2024

@NeerajNagure It seems like something went wrong with your rebase. 😄 This PR now has 21 unrelated commits.

@NeerajNagure
added template name to error
0c09a86 
Signed-off-by: Neeraj Nagure <nagureneeraj@gmail.com>
@NeerajNagure
renamed errApplyComposed and fixed unit tests
1a11084 
Signed-off-by: Neeraj Nagure <nagureneeraj@gmail.com>
@NeerajNagure
Copy link
Contributor Author

@negz I have fixed all the issues related to rebase.Please check it and inform me any other changes are supposed to be made

@NeerajNagure
fixed protoc-gen-go version
f5bebfd 
Signed-off-by: Neeraj Nagure <nagureneeraj@gmail.com>
@NeerajNagure NeerajNagure requested a review from jbw976 May 22, 2024 04:47
@NeerajNagure
fixed run_function.pb.go
8e08fbf 
Signed-off-by: Neeraj Nagure <nagureneeraj@gmail.com>
@NeerajNagure NeerajNagure force-pushed the enhance-error-logging branch 2 times, most recently from d3f6473 to 8e08fbf Compare May 22, 2024 05:03
negz
negz approved these changes May 22, 2024
View reviewed changes
Copy link
Member

@negz negz left a comment

Choose a reason for hiding this comment

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

Thanks for persisting! This looks great.

@negz negz merged commit a8b579d into crossplane:master May 22, 2024
16 of 17 checks passed
@NeerajNagure NeerajNagure deleted the enhance-error-logging branch May 31, 2024 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@negz negz negz approved these changes

@Pocokwins Pocokwins Pocokwins approved these changes

@bobh66 bobh66 Awaiting requested review from bobh66 bobh66 is a code owner automatically assigned from crossplane/crossplane-maintainers

@bassam bassam Awaiting requested review from bassam bassam was automatically assigned from crossplane/steering-committee

@jbw976 jbw976 Awaiting requested review from jbw976

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

enhance error logging for existing object is not controlled by UID - add GVK, Resource Name etc.
4 participants
@NeerajNagure @negz @jbw976 @Pocokwins