-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
fix(integrations): update usage of is_enabled data forwarding #103584
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
base: master
Are you sure you want to change the base?
Conversation
❌ 2 Tests Failed:
View the top 2 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
leeandher
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, may just want a test for the new models to prevent a regression
|
|
||
| def validate_config(self, config) -> SQSConfig | SegmentConfig | SplunkConfig: | ||
| # Filter out empty string values (cleared optional fields) | ||
| config = {k: v for k, v in config.items() if v != ""} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Config filter permits whitespace-only credential values
The filter removes empty string values but allows whitespace-only strings like " " to pass through. Since allow_blank=True was added to the config field, values that are only spaces now pass the filter and reach validation without being rejected. This allows invalid credentials (whitespace-only access_key or secret_key) to pass serializer validation and fail later at runtime. The filter should check for stripped-empty values.
| assert response.data["provider"] == DataForwarderProviderSlug.SEGMENT | ||
|
|
||
| data_forwarder = DataForwarder.objects.get(id=response.data["id"]) | ||
| assert data_forwarder.project.count() == 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Test accesses wrong related_name for data_forwarder
The test accesses data_forwarder.project.count() but the DataForwarderProject model defines the reverse relationship with related_name="projects". This should be data_forwarder.projects.count() to correctly reference the related manager and avoid an AttributeError at runtime.
ECO 1200 and ECO 1269