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

feat: webhook v2 spec upgrade #5224

Merged
merged 20 commits into from
Jan 9, 2025
Merged

Conversation

vinayteki95
Copy link
Contributor

@vinayteki95 vinayteki95 commented Oct 24, 2024

Description

  • V2 adapter is introduced to send v2 spec events
  • Source transformer payloads will be built differently for different specs (v1 and v2)
  • Features service will be aware of v2 flag from transformer and pick adapter accordingly

Linear Ticket

Resolves INT-2802

Security

  • The code changed/added as part of this pull request won't create any security issues with how the software is being used.

@vinayteki95 vinayteki95 changed the title Feat.webhook v2 spec upgrade feat: webhook v2 spec upgrade Oct 24, 2024
@vinayteki95 vinayteki95 force-pushed the feat.webhook-v2-spec-upgrade branch from 8df117d to 42a0c5b Compare October 28, 2024 10:47
Copy link

codecov bot commented Oct 29, 2024

Codecov Report

Attention: Patch coverage is 91.30435% with 6 lines in your changes missing coverage. Please review.

Project coverage is 74.76%. Comparing base (ddf04ff) to head (ea8e322).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
services/transformer/features_impl.go 25.00% 2 Missing and 1 partial ⚠️
services/transformer/features.go 0.00% 2 Missing ⚠️
gateway/webhook/webhook.go 90.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5224      +/-   ##
==========================================
- Coverage   74.83%   74.76%   -0.07%     
==========================================
  Files         440      440              
  Lines       61528    61570      +42     
==========================================
- Hits        46044    46033      -11     
- Misses      12958    12998      +40     
- Partials     2526     2539      +13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@vinayteki95 vinayteki95 requested a review from koladilip October 29, 2024 06:53
@vinayteki95 vinayteki95 marked this pull request as ready for review November 4, 2024 09:50
@vinayteki95 vinayteki95 requested a review from sanpj2292 November 4, 2024 09:52
@@ -74,8 +76,8 @@ func (*noopService) Regulations() []string {
}

func (*noopService) SourceTransformerVersion() string {
// v0 is deprecated
return V1
// v0 is deprecated and upgrading to v2
Copy link
Contributor

Choose a reason for hiding this comment

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

features_impl.go returns version based on upgradedToSourceTransformV2 value received by /features response from transformer

when does this come into picture ? and what difference it makes returning v1 vs. v2 here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is used in some test cases as a placeholder for transformer service. This used to return v0 earlier but as v0 is deprecated v1 is the ideal response.
This doesn't affect the actual logic anywhere.

@ktgowtham ktgowtham requested a review from mihir20 December 26, 2024 10:36
@ktgowtham ktgowtham self-requested a review December 30, 2024 11:10
Copy link
Contributor

@mihir20 mihir20 left a comment

Choose a reason for hiding this comment

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

LGTM

@vinayteki95 vinayteki95 merged commit 92ba9ce into master Jan 9, 2025
56 checks passed
@vinayteki95 vinayteki95 deleted the feat.webhook-v2-spec-upgrade branch January 9, 2025 06:06
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.

4 participants