Skip to content

Add URL filtering#72

Open
Voamorim wants to merge 4 commits intomainfrom
urls_filter
Open

Add URL filtering#72
Voamorim wants to merge 4 commits intomainfrom
urls_filter

Conversation

@Voamorim
Copy link

No description provided.

Copy link
Member

@cunha cunha left a comment

Choose a reason for hiding this comment

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

Main points:

  • Tests don't match expected behavior; we need to define a number of domain and path levels beyond which we start "anonymization"
  • Need tests to cover URL query parameters
  • Need tests to cover special domains and CC TLDs

Copy link
Member

@cunha cunha left a comment

Choose a reason for hiding this comment

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

Forgot to mention CC TLDs in the previous review.

Copy link
Member

@cunha cunha left a comment

Choose a reason for hiding this comment

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

Another round of comments. I think we should focus on comparing the URL identity keys (after we "erase" subdomains, path levels, and query parameter values). The filter is just taking one URL for each distinct identity key.

Copy link
Member

@cunha cunha left a comment

Choose a reason for hiding this comment

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

Requesting a few more additional testes around ccTLDs.

@Voamorim Voamorim requested a review from cunha March 1, 2026 22:01
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