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.

Sign up for GitHub

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

Jump to bottom

Frequency cap recording #107

Merged
merged 14 commits into from
Oct 24, 2024
Merged

Frequency cap recording #107

merged 14 commits into from
Oct 24, 2024

Conversation

kookster
Copy link
Member

@kookster kookster commented Jun 10, 2024 •
edited
Loading

  • Record idempotent concurrency safe impressions to ddb
  • Evict old impression dates
  • Bug - don't record impressions older than the max horizon of the freq cap
  • Define frequency as its own mode, so it gets separate processing
  • Filter out records with no freq capped impressions
  • Delete based on the longest freq cap "days"
  • Only delete when there are > 50% of records that are unneeded
  • Tests with local ddb? Or with staging?
  • Add to start scripts in package.json
  • Update README
  • Add Cfn entry for Frequency as its own function (Template updates for Frequency Capping Infrastructure#784)
  • Ensure Cfn roles and permissions include dynamodb access to the frequency table

@kookster kookster changed the title freq cap Frequency cap recording Jun 10, 2024
cavis
cavis reviewed Jun 11, 2024
View reviewed changes
Copy link
Member

@cavis cavis left a comment

Choose a reason for hiding this comment

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

The questions!

lib/inputs/dovetail-frequency.js Outdated Show resolved Hide resolved
lib/inputs/dovetail-frequency.js Show resolved Hide resolved
lib/inputs/dovetail-frequency.js Show resolved Hide resolved
@kookster kookster changed the title Frequency cap recording WIP - Frequency cap recording Jun 13, 2024
@kookster kookster marked this pull request as draft September 21, 2024 21:02
@kookster kookster changed the title WIP - Frequency cap recording Frequency cap recording Sep 21, 2024
@kookster kookster marked this pull request as ready for review September 22, 2024 14:59
@kookster kookster requested a review from cavis September 22, 2024 14:59
cavis
cavis approved these changes Sep 23, 2024
View reviewed changes
Copy link
Member

@cavis cavis left a comment

Choose a reason for hiding this comment

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

Some last questions for my own understanding... but I think this looks good! 👍

lib/inputs/dovetail-frequency.js Show resolved Hide resolved
lib/inputs/dovetail-frequency.js Show resolved Hide resolved
@kookster kookster merged commit a18ab4b into main Oct 24, 2024
2 checks passed
@kookster kookster deleted the feat/freq_cap branch October 24, 2024 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cavis cavis cavis approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kookster @cavis