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

Adding Service Hub HTTP Middleware to NPS Client #691

Merged
merged 3 commits into from
Feb 20, 2025
Merged

Adding Service Hub HTTP Middleware to NPS Client #691

merged 3 commits into from
Feb 20, 2025
+30 −19

Conversation

toma3233
Copy link
Contributor

Fixes #

Description
This PR adds the Service Hub HTTP Middleware to the NPS Client.

The middleware used can be found here: https://github.com/Azure/aks-middleware/blob/main/http/client/direct/restlogger/restlogger.go

How was this change tested?
AKS E2E tests
*

Does this change impact docs?

  • Yes, PR includes docs updates
  • Yes, issue opened: #
  • No

Release Note

@toma3233 toma3233 requested a review from comtalyst February 12, 2025 00:41
@Bryce-Soghigian
Copy link
Collaborator 

New file modification detected in the Git working tree. Please check in before commit.
  - charts/karpenter-crd/templates/karpenter.azure.com_aksnodeclasses.yaml
  - pkg/apis/crds/karpenter.azure.com_aksnodeclasses.yaml
  - pkg/providers/imagefamily/customscriptsbootstrap/provisionclientbootstrap.go

CI Test is failing, you need to run make presubmit

comtalyst
comtalyst previously approved these changes Feb 20, 2025
View reviewed changes
@toma3233 toma3233 dismissed stale reviews from comtalyst and Bryce-Soghigian via 6fa7524 February 20, 2025 01:17
@toma3233 toma3233 force-pushed the toma/addLoggingMiddleware branch from 6af805e to 6fa7524 Compare February 20, 2025 01:17
Bryce-Soghigian
Bryce-Soghigian requested changes Feb 20, 2025
View reviewed changes
Copy link
Collaborator

@Bryce-Soghigian Bryce-Soghigian left a comment

Choose a reason for hiding this comment

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

i dont like slog

Bryce-Soghigian
Bryce-Soghigian previously approved these changes Feb 20, 2025
View reviewed changes
Copy link
Collaborator

@Bryce-Soghigian Bryce-Soghigian left a comment

Choose a reason for hiding this comment

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

Im just kidding looks good lol

@toma3233 toma3233 merged commit bbcfe96 into main Feb 20, 2025
11 of 12 checks passed
@toma3233 toma3233 deleted the toma/addLoggingMiddleware branch February 20, 2025 01:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Bryce-Soghigian Bryce-Soghigian Bryce-Soghigian approved these changes

@comtalyst comtalyst comtalyst left review comments

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

Successfully merging this pull request may close these issues.
3 participants
@toma3233 @Bryce-Soghigian @comtalyst