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

fix: remove support of event stream #1804

Merged
merged 2 commits into from
Nov 19, 2024
Merged

Conversation

ItsSudip
Copy link
Member

@ItsSudip ItsSudip commented Nov 18, 2024

What are the changes introduced in this PR?

We are removing support of event stream.

What is the related Linear task?

Resolves INT-XXX

Please explain the objectives of your changes below

Put down any required details on the broader aspect of your changes. If there are any dependent changes, mandatorily mention them here

Any changes to existing capabilities/behaviour, mention the reason & what are the changes ?

N/A

Any new dependencies introduced with this change?

N/A

Any new checks got introduced or modified in test suites. Please explain the changes.

N/A


Developer checklist

  • My code follows the style guidelines of this project

  • No breaking changes are being introduced.

  • All related docs linked with the PR?

  • All changes manually tested?

  • Any documentation changes needed with this change?

  • I have executed schemaGenerator tests and updated schema if needed

  • Are sensitive fields marked as secret in definition config?

  • My test cases and placeholders use only masked/sample values for sensitive fields

  • Is the PR limited to 10 file changes & one task?

Reviewer checklist

  • Is the type of change in the PR title appropriate as per the changes?

  • Verified that there are no credentials or confidential data exposed with the changes.

Summary by CodeRabbit

  • New Features

    • Updated configuration for the LinkedIn Audience destination to support only warehouse connections.
    • Removed cloud-related properties from the schema for cookie categories and consent purposes.
  • Bug Fixes

    • Ensured that only relevant properties are retained in the configuration and schema, improving clarity and functionality.

Copy link

coderabbitai bot commented Nov 18, 2024

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

The pull request modifies the configuration files for the "LINKEDIN_AUDIENCE" destination. Changes in db-config.json include the removal of the "cloud" entry from both supportedSourceTypes and supportedConnectionModes, as well as the elimination of the "cloud" key from the defaultConfig array. In schema.json, the "cloud" property is removed from the "oneTrustCookieCategories" and "ketchConsentPurposes" objects, while the "warehouse" property remains unchanged.

Changes

File Path Change Summary
src/configurations/destinations/linkedin_audience/... - Removed "cloud" from supportedSourceTypes and supportedConnectionModes in db-config.json.
- Eliminated "cloud" key from defaultConfig in db-config.json.
src/configurations/destinations/linkedin_audience/... - Removed "cloud" property from oneTrustCookieCategories and ketchConsentPurposes in schema.json.

Possibly related PRs

  • feat: onboard linkedin audience destination #1758: The changes in the main PR regarding the removal of the "cloud" key in the db-config.json and schema.json files are directly related to the onboarding of the LinkedIn Audience destination, which also involves similar configurations in its db-config.json and schema.json files.

Suggested reviewers

  • lvrach
  • am6010
  • AchuthaSourabhC
  • ssbeefeater
  • debanjan97
  • cisse21
  • ruchiramoitra
  • 1abhishekpandey
  • sandeepdsvs
  • koladilip
  • aashishmalik
  • shrouti1507

🐇 In the fields where rabbits play,
Changes hop in a brand new way.
"Cloud" has vanished, "warehouse" stays,
Configs now sing in clearer ways!
Let's celebrate with a joyful cheer,
For LinkedIn's path is now so clear! 🌼


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

codecov bot commented Nov 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (a039627) to head (d7418e4).
Report is 2 commits behind head on release/v1.98.0.

Additional details and impacted files
@@                Coverage Diff                @@
##           release/v1.98.0     #1804   +/-   ##
=================================================
  Coverage           100.00%   100.00%           
=================================================
  Files                    2         2           
  Lines                   53        53           
  Branches                 7         7           
=================================================
  Hits                    53        53           

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


🚨 Try these New Features:

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between a039627 and cda8b2b.

📒 Files selected for processing (2)
  • src/configurations/destinations/linkedin_audience/db-config.json (1 hunks)
  • src/configurations/destinations/linkedin_audience/schema.json (0 hunks)
💤 Files with no reviewable changes (1)
  • src/configurations/destinations/linkedin_audience/schema.json
🔇 Additional comments (1)
src/configurations/destinations/linkedin_audience/db-config.json (1)

Line range hint 1-29: Verify impact on existing integrations

Since this change removes cloud support, we should ensure this won't break existing integrations.

#!/bin/bash
# Description: Check for potential breaking changes

# Search for references to LinkedIn Audience cloud mode in other configuration files
echo "Checking references to LinkedIn Audience cloud mode..."
rg -l "LINKEDIN_AUDIENCE.*cloud" src/configurations/

# Check if there are any migration guides or documentation that need updating
echo "Checking for documentation that might need updates..."
fd -t f . docs -e md -e mdx | xargs rg -l "LINKEDIN_AUDIENCE.*cloud"

Comment on lines +13 to 24
"supportedSourceTypes": ["warehouse"],
"supportedMessageTypes": {
"cloud": ["record"]
},
"supportedConnectionModes": {
"cloud": ["cloud"],
"warehouse": ["cloud"]
},
"destConfig": {
"defaultConfig": ["rudderAccountId"],
"cloud": ["connectionMode", "oneTrustCookieCategories", "ketchConsentPurposes"],
"warehouse": ["connectionMode", "oneTrustCookieCategories", "ketchConsentPurposes"]
},
"secretKeys": []
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

⚠️ Potential issue

Cloud mode references should be removed for consistency

The verification confirms that cloud-related configurations should be removed from db-config.json for consistency:

  1. supportedSourceTypes already excludes "cloud", indicating the intended direction
  2. No cloud-related configurations exist in schema.json
  3. ui-config.json only shows warehouse-related configurations
  4. The remaining cloud references in db-config.json appear to be orphaned:
    - "supportedMessageTypes": {
    -   "cloud": ["record"]
    - },
    - "supportedConnectionModes": {
    -   "warehouse": ["cloud"]
    - },
🔗 Analysis chain

Inconsistent removal of cloud-related configurations

While "cloud" has been removed from supportedSourceTypes, it's still referenced in:

  1. supportedMessageTypes.cloud
  2. supportedConnectionModes.warehouse: ["cloud"]

This partial removal could lead to confusion and potential runtime issues.

To ensure consistency, please verify if these configurations should also be updated:

  "supportedSourceTypes": ["warehouse"],
  "supportedMessageTypes": {
-   "cloud": ["record"]
  },
  "supportedConnectionModes": {
    "warehouse": ["cloud"]
  },

Let's verify other files for similar patterns:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for other cloud-related configurations in LinkedIn Audience integration

# Search for cloud-related configurations in LinkedIn Audience files
echo "Searching for cloud configurations in LinkedIn Audience files..."
rg -l "cloud" src/configurations/destinations/linkedin_audience/

# Check schema.json for related configurations
echo "Checking schema.json for related configurations..."
if [ -f "src/configurations/destinations/linkedin_audience/schema.json" ]; then
  jq 'paths | select(.[] | tostring | contains("cloud"))' src/configurations/destinations/linkedin_audience/schema.json
fi

Length of output: 548


Script:

#!/bin/bash
# Let's try a more focused search approach

# 1. Check for cloud-related configurations in the specific file
echo "Content of db-config.json:"
cat src/configurations/destinations/linkedin_audience/db-config.json

# 2. Search for any cloud-related patterns in the linkedin_audience directory
echo -e "\nSearching for 'cloud' in linkedin_audience directory:"
rg "cloud" src/configurations/destinations/linkedin_audience/

# 3. Look for any other config files in the linkedin_audience directory
echo -e "\nListing all files in linkedin_audience directory:"
fd . src/configurations/destinations/linkedin_audience/

Length of output: 1571


Script:

#!/bin/bash
# Let's check the schema.json and ui-config.json for any cloud-related configurations

# Check schema.json content
echo "Content of schema.json:"
cat src/configurations/destinations/linkedin_audience/schema.json

# Check ui-config.json content
echo -e "\nContent of ui-config.json:"
cat src/configurations/destinations/linkedin_audience/ui-config.json

# Search for any connection mode related patterns
echo -e "\nSearching for connectionMode patterns:"
rg -A 3 -B 3 "connectionMode" src/configurations/destinations/linkedin_audience/

Length of output: 7225

@ItsSudip ItsSudip changed the base branch from develop to release/v1.98.0 November 19, 2024 02:55
@ItsSudip ItsSudip merged commit 5eaed1a into release/v1.98.0 Nov 19, 2024
11 checks passed
@ItsSudip ItsSudip deleted the fix.linkedin_audience branch November 19, 2024 05:41
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.

2 participants