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

Update AutoPull - Temporary fix for #1806 #1809

Closed
wants to merge 1 commit into from

Conversation

dnsguru
Copy link
Member

@dnsguru dnsguru commented Jul 26, 2023

This updates newtlds.go to replace the with "Contracted" in the header per 2012 round nTLD as a tempfix to #1806

Short version of #1806 is that now we're 10 years ++ hence from the 2012 round TLD launches, the contracts are hitting their 10 year horizon and are being renewed. ICANN is updating the contract date field in the gtlds.json file, and this would result in a large quantity of auto-pull to merge with the new dates as this happens.

There are discussions with ICANN's technical team to see about them leaving the contract dates intact in contractdate vs re-using the contractdate field to store the contract renewal date.

It is anticipated that this pull request being merged will cause one large pull request that alters the entire nTLD section's contracted dates in the comment per nTLD, but will then reduce the subsequent noise level when ICANN renews contracts with the various Registries.

@dnsguru dnsguru added r=weppos Marked as approved and ready to merge by @weppos NOT IOS FB Submitter attests PR is not #1245 related ✅ autopull Automation pull from Authoritative ICANN json source 🚩ICANN (IANA/ICP-3) Section PR changes in the ICANN/IANA section typically reserved for TLDs. non .dat Change or Coding Review Alteration to code/automation or publicsuffix.org site labels Jul 26, 2023
@dnsguru dnsguru requested a review from weppos July 26, 2023 17:27
Copy link
Contributor

@cpu cpu left a comment

Choose a reason for hiding this comment

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

What about deleting the DateOfContractSignature field instead of assigning a constant value to it?

e.g. on L146, change to in-lining the "Contracted" constant:

e.DateOfContractSignature,

Then we can delete L121 and L103, L104.

As written the "Contracted" constant will be normalized to "contracted" and the field has no variability in possible values.

@cpu
Copy link
Contributor

cpu commented Jul 26, 2023

(If it's easier I can open a PR implementing my suggested feedback in a couple hours)

@cpu
Copy link
Contributor

cpu commented Jul 26, 2023

There's actually more work to do... It looks like the Go tool unit tests aren't being run as part of pull request status checks anymore, just the cronjob that runs the code in master. We can tell that's the case because your change would have broken the unit tests and the branch status is reporting all green :-/

The Go tests used to be run explicitly via Travis for all pull requests:

- go test -v -coverprofile=coverage.out tools/*.go

The new GitHub actions replacement for Travis that was added in #1750 only runs the Makefile tests:

- name: Run test
run: make test

@cpu
Copy link
Contributor

cpu commented Jul 26, 2023

PTAL at #1812 I think it will achieve the same goals as this branch in a tidier way.

@weppos
Copy link
Member

weppos commented Aug 1, 2023

Closing in favor of #1812

@weppos weppos closed this Aug 1, 2023
@weppos weppos deleted the autopull-contractdate-tempfix branch August 1, 2023 08:31
@dnsguru
Copy link
Member Author

dnsguru commented Aug 7, 2023

I have just been notified by ICANN's Technical Services that they will preserve the contract date of original signing.

So now the question will be, "do we re-introduce the date?"

Rather than reintroducing it, I think instead introducing some human-readable datestamp in the .dat header would solve the problem that the contract date was helping address - troubleshooting time-lag where folks were not updating their file as TLDs were being added to the zone file.

@weppos
Copy link
Member

weppos commented Aug 8, 2023

So now the question will be, "do we re-introduce the date?"

How meaningful was that entry? Personally, I don't recall if we ever had it for a particular purpose. I don't feel any pressure to reintroduce it.

@dnsguru
Copy link
Member Author

dnsguru commented Aug 8, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✅ autopull Automation pull from Authoritative ICANN json source 🚩ICANN (IANA/ICP-3) Section PR changes in the ICANN/IANA section typically reserved for TLDs. non .dat Change or Coding Review Alteration to code/automation or publicsuffix.org site NOT IOS FB Submitter attests PR is not #1245 related r=weppos Marked as approved and ready to merge by @weppos
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants