Skip to content

Comments

Remove null=True for DNSDomain and DNSRecords#133

Closed
peterbppb wants to merge 1 commit intomainfrom
dns_ips_models
Closed

Remove null=True for DNSDomain and DNSRecords#133
peterbppb wants to merge 1 commit intomainfrom
dns_ips_models

Conversation

@peterbppb
Copy link
Contributor

dns_ips slack module assumes domains and records cannot be null so it tries to strip them here: https://github.com/surface-security/surface/blob/main/surface/dns_ips/slack.py#L74-83

We should either remove null=True to avoid this issue or filter for that in the slack processor.

I think removing null=True from these model fields makes sense as I do not think we have any need for records/domains without a name.

@peterbppb peterbppb requested a review from a team as a code owner June 28, 2023 11:11
@peterbppb peterbppb requested review from DDuarte and fopina June 28, 2023 11:11
Copy link
Contributor

@gsilvapt gsilvapt left a comment

Choose a reason for hiding this comment

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

I'm in favour of removing these attributes in both models, they are not appropriate for their data types.
When releasing this, we might need to add a note for all forks take into account this change (they will need to provide a default, most likely).

@codecov
Copy link

codecov bot commented Mar 19, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.27%. Comparing base (8e4f55f) to head (f994e3a).
Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #133      +/-   ##
==========================================
+ Coverage   77.25%   77.27%   +0.02%     
==========================================
  Files          84       85       +1     
  Lines        4022     4026       +4     
==========================================
+ Hits         3107     3111       +4     
  Misses        915      915              

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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