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

IP_ADDRESS_CACHE cache improvements #501

Merged
merged 1 commit into from
Apr 1, 2024

Conversation

rodpayne
Copy link
Contributor

  • Only add entries to the cache if the lookup was successful. (The ones that fail may work next time.)
  • Retain IP_ADDRESS_CACHE entries for up to four hours. (It takes almost a day to import a day's reports, so the lookups don't need to be extremely fresh.)

Copy link

codecov bot commented Mar 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 59.93%. Comparing base (041296b) to head (dc53ca2).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #501      +/-   ##
==========================================
+ Coverage   59.84%   59.93%   +0.08%     
==========================================
  Files          12       12              
  Lines        1574     1575       +1     
==========================================
+ Hits          942      944       +2     
+ Misses        632      631       -1     

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

@@ -48,7 +48,7 @@
MAGIC_XML = b"\x3c\x3f\x78\x6d\x6c\x20"
MAGIC_JSON = b"\7b"

IP_ADDRESS_CACHE = ExpiringDict(max_len=10000, max_age_seconds=1800)
IP_ADDRESS_CACHE = ExpiringDict(max_len=10000, max_age_seconds=14400)
Copy link
Contributor

Choose a reason for hiding this comment

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

It may worth making this a config file setting, defaulting to the initial 1800, as different use-cases may warrant different values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, making it a config file setting may be a good enhancement, but it may take me a while to figure out how to do it. I can take a look at it after the holiday.

I don't think that there are currently many people locked into the 1,800 setting since it was not working at all until last week.

Copy link
Contributor

Choose a reason for hiding this comment

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

If someone would like to contribute a PR to add that option, be aware that although IP_ADDRESS_CACHE is only referenced twice:

https://github.com/domainaware/parsedmarc/blob/master/parsedmarc/__init__.py#L103

https://github.com/domainaware/parsedmarc/blob/master/parsedmarc/__init__.py#L897

configuration items related to report processing must go through several layers of function calls, such as when I just added the new function calls to address the concerns raised in #500.

041296b...acef7bd

@seanthegeek seanthegeek merged commit 8936193 into domainaware:master Apr 1, 2024
3 checks passed
seanthegeek added a commit that referenced this pull request Apr 2, 2024
- Actually save `source_type` and `source_name` to Elasticsearch and OpenSearch
- Reverse-lookup cache improvements (PR #501 closes issue #498)
- Update the included `dbip-country-lite.mmdb` to the 2024-03 version
- Update `base_reverse_dns_map.csv`
- Add new general config options (closes issue #500)
  - `always_use_local_files` - Disables the download of the reverse DNS map
  - `local_reverse_dns_map_path` - Overrides the default local file path to use for the reverse DNS map
  - `reverse_dns_map_url` - Overrides the default download URL for the reverse DNS map
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.

3 participants