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

chore: default webhook contract version to v2 #5659

Merged
merged 4 commits into from
Apr 4, 2025

Conversation

vinayteki95
Copy link
Contributor

Description

As part of complete migration to webhook v2 contract, we are defaulting to v2 when features service is not ready yet.

Linear Ticket

Resolves INT-2757
https://linear.app/rudderstack/issue/INT-2757/stage-3-or-clean-up-or-no-more-adapters-to-convert-v2-v1-spec

Security

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

Copy link

codecov bot commented Mar 28, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.89%. Comparing base (a38b4f5) to head (7371767).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5659      +/-   ##
==========================================
- Coverage   77.04%   76.89%   -0.15%     
==========================================
  Files         483      483              
  Lines       65534    65534              
==========================================
- Hits        50491    50393      -98     
- Misses      12295    12376      +81     
- Partials     2748     2765      +17     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mihir20 mihir20 requested review from mihir20 and Copilot April 3, 2025 05:33
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the webhook contract defaults as part of the migration to webhook v2 by enabling the upgraded source transform flag.

  • In testhelper/transformertest/builder.go, the JSON payload is updated to include "upgradedToSourceTransformV2": true.
  • In services/transformer/features.go, the default feature configuration for v2 is applied.
  • In services/transformer/features_impl_test.go and regulation-worker/cmd/main_test.go, the tests are updated to reflect the new default source transformer version.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
testhelper/transformertest/builder.go Updated JSON features payload to include v2 flag
services/transformer/features_impl_test.go Modified test expectations to use v2 as the default version
services/transformer/features.go Changed default JSON configuration to set upgradedToSourceTransformV2 true
regulation-worker/cmd/main_test.go Updated test fixture to include the new v2 feature flag
Comments suppressed due to low confidence (2)

testhelper/transformertest/builder.go:137

  • Ensure that the new feature flag 'upgradedToSourceTransformV2' added in this test helper is complemented by tests that verify the expected behavior in production code.
features := []byte(`{"routerTransform": {}, "supportSourceTransformV1": true, "upgradedToSourceTransformV2": true}`)

services/transformer/features_impl_test.go:108

  • [nitpick] Consider adding additional test cases to verify the behavior when only one of the source transform flags is set, to ensure that all edge cases are clearly handled.
It("If source transform is not v1 or v2, it should panic as v0 is deprecated", func() {

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

@ktgowtham ktgowtham self-requested a review April 3, 2025 11:38
@vinayteki95 vinayteki95 merged commit 7dbf5d8 into master Apr 4, 2025
58 checks passed
@vinayteki95 vinayteki95 deleted the chore.default-to-v2-webhook branch April 4, 2025 06:56
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