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

Normalize domain column #1005

Merged
6 commits merged into from May 16, 2022
Merged

Normalize domain column #1005

6 commits merged into from May 16, 2022

Conversation

ghost
Copy link

@ghost ghost commented May 12, 2022

Purpose

Store the normalized domain value in the "domain" column in database.

Context

Follow-up of #983

Changes

Database files.
Normalize passed "domain" in get_test_history() before retrieval (since domains are now normalized in database).
Update the upgrade script.

How to test this PR

Unit tests are added, they should pass. Or start a test with a trailing dot to the domain (and/or with uppercase letters), the domain stored in database should be normalized.

Using the upgrade script should remove any trailing dot from the domain column, as well as down casing it.

@ghost ghost added this to the v2022.1 milestone May 12, 2022
@ghost ghost requested review from mattias-p, matsduf and hannaeko May 12, 2022 15:40
@ghost
Copy link
Author

ghost commented May 12, 2022

I guess this requires a database update to normalize all stored domains. This could be done in a second stage, once this PR is approved.

@matsduf matsduf added the T-Bug Type: Bug in software or error in test case description label May 12, 2022
matsduf
matsduf previously approved these changes May 12, 2022
@matsduf
Copy link
Contributor

matsduf commented May 12, 2022

I guess this requires a database update to normalize all stored domains. This could be done in a second stage, once this PR is approved.

It would be nice, including down-casing the domain name, it is however, not strictly necessary by this PR. The issue of domain names being stored with and without trailing dot is an old issue.

Alexandre Pion added 6 commits May 16, 2022 11:51
With PR #798 we normalize the way we store the "params" data in
database. The "domain" column should also be normalized, especially
since it's used by get_test_history() method.
@ghost
Copy link
Author

ghost commented May 16, 2022

I updated the upgrade script since this change would alter how get_test_history() works. With this change some tests might be never reachable if the database is not updated.

@ghost ghost requested a review from matsduf May 16, 2022 09:53
Copy link
Member

@mattias-p mattias-p left a comment

Choose a reason for hiding this comment

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

LGTM

@ghost ghost merged commit aaefb6b into zonemaster:develop May 16, 2022
@mattias-p
Copy link
Member

I found a problem during release testing and I created #1030 to fix it.

@mattias-p
Copy link
Member

After the fix in #1030 I successfully tested this for v2022.1.

@mattias-p mattias-p added the S-ReleaseTested Status: The PR has been successfully tested in release testing label Jun 2, 2022
@ghost ghost deleted the normalize-domain-column branch December 23, 2022 09:58
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-ReleaseTested Status: The PR has been successfully tested in release testing T-Bug Type: Bug in software or error in test case description
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants