-
Notifications
You must be signed in to change notification settings - Fork 285
Add an "about" link to registrars in RDAP #2771
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
Conversation
5d11f3e
to
7b5399c
Compare
@CydeWeys friendly ping |
This corresponds to the Feb 2024 response profile section 1.2 and implementation guide 1.3 respectively, now that we comply (or are, at least closer to complying), with the Feb 2024 versions. This should probably depend on google#2771 because that includes a small change included in the Feb 2024 version
This corresponds to the Feb 2024 response profile section 1.2 and implementation guide 1.3 respectively, now that we comply (or are, at least closer to complying), with the Feb 2024 versions. This should probably depend on google#2771 because that includes a small change included in the Feb 2024 version
This corresponds to the Feb 2024 response profile section 1.2 and implementation guide 1.3 respectively, now that we comply (or are, at least closer to complying), with the Feb 2024 versions. This should probably depend on google#2771 because that includes a small change included in the Feb 2024 version This also updates the documentation to reference the proper areas of the specifications.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 37 files reviewed, 2 unresolved discussions (waiting on @CydeWeys)
core/src/main/java/google/registry/rdap/UpdateRegistrarRdapBaseUrlsAction.java
line 83 at r1 (raw file):
private static void processAllRegistrars(ImmutableMap<String, String> ianaIdsToUrls) { int nonUpdatedRegistrars = 0; // Split into multiple transactions to avoid load-save-reload conflicts. Re-building a registrar
Not clear what this means. I don't see reloads in the code flow.
Code quote:
load-save-reload
core/src/main/java/google/registry/rdap/UpdateRegistrarRdapBaseUrlsAction.java
line 85 at r1 (raw file):
// Split into multiple transactions to avoid load-save-reload conflicts. Re-building a registrar // requires a full (cached) load of all registrars to avoid IANA ID conflicts. Iterable<Registrar> registrars = tm().transact(Registrar::loadAll);
Use transact(REPEATABLE_READ, callable)
can reduce race.
Code quote:
tm().transact(
From the response profile: 2.4.6. Registrar URL - The entity with the registrar role in the RDAP response MUST contain a links member [RFC9083]. The links object MUST contain the elements: value, identical to the the RDAP Base URL for the Registrar as provided in the IANA “Registrar IDs” registry (i.e., https://www.iana.org/assignments/registrar-ids); rel:about, and href containing the Registrar URL. Note: in cases where the Registry Operator acts as sponsoring Registrar (e.g., IANA Registrar ID 9999), the href shall contain a URL from the Registry.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 37 files reviewed, 2 unresolved discussions (waiting on @CydeWeys and @weiminyu)
core/src/main/java/google/registry/rdap/UpdateRegistrarRdapBaseUrlsAction.java
line 83 at r1 (raw file):
Previously, weiminyu (Weimin Yu) wrote…
Not clear what this means. I don't see reloads in the code flow.
not directly, but as the comment notes, the Registrar.Builder::build method requires reloading all registrars to avoid IANA ID conflicts.
So, in the previous all-in-one-transaction code path, it throws an error if more than one registrar is modified -- because the firstly-changed registrar is reloaded in the "build" method of the secondly-changed registrar.
We never saw this error in practice because we never encountered the case where more than one registrar URL changed at a time.
core/src/main/java/google/registry/rdap/UpdateRegistrarRdapBaseUrlsAction.java
line 85 at r1 (raw file):
Previously, weiminyu (Weimin Yu) wrote…
Use
transact(REPEATABLE_READ, callable)
can reduce race.
sure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 37 files reviewed, 1 unresolved discussion (waiting on @CydeWeys)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 35 of 37 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @CydeWeys)
This corresponds to the Feb 2024 response profile section 1.2 and implementation guide 1.3 respectively, now that we comply (or are, at least closer to complying), with the Feb 2024 versions. This should probably depend on google#2771 because that includes a small change included in the Feb 2024 version This also updates the documentation to reference the proper areas of the specifications.
This corresponds to the Feb 2024 response profile section 1.2 and implementation guide 1.3 respectively, now that we comply (or are, at least closer to complying), with the Feb 2024 versions. This should probably depend on #2771 because that includes a small change included in the Feb 2024 version This also updates the documentation to reference the proper areas of the specifications.
From the response profile:
2.4.6. Registrar URL - The entity with the registrar role in the RDAP response MUST contain a links member [RFC9083]. The links object MUST contain the elements: value, identical to the the RDAP Base URL for the Registrar as provided in the IANA “Registrar IDs” registry (i.e., https://www.iana.org/assignments/registrar-ids); rel:about, and href containing the Registrar URL. Note: in cases where the Registry Operator acts as sponsoring Registrar (e.g., IANA Registrar ID 9999), the href shall contain a URL from the Registry.
This change is