#4152: Use httpx as the api library for dns hosting #4187

kimallen · 2025-09-16T17:49:53Z

Ticket

Resolves #4152

Changes

Replaces api library from requests to httpx in the CloudflareService methods
Fixes bug where updating nameservers was crashing

Context for reviewers

I'm not sure the best pattern for error handling/raising errors, and the error calls could be more granular. I'm open to suggestions, or this can be something we tackle in more depth later (both handling of errors and bubbling up of errors)

Setup

Should be able to use add a dns record to a domain that hasn't had one before and also new records to accounts/zones that already exist. See instructions for running locally
or test in sandbox kma (can use instructions from this PR to test with a new domain name)

Code Review Verification Steps

As the original developer, I have

Satisfied acceptance criteria and met development standards

Met the acceptance criteria, or will meet them in a subsequent PR
Created/modified automated tests
Update documentation in READMEs and/or onboarding guide

Ensured code standards are met (Original Developer)

If any updated dependencies on Pipfile, also update dependencies in requirements.txt.
Interactions with external systems are wrapped in try/except
Error handling exists for unusual or missing values

Validated user-facing changes (if applicable)

Follow the process for requesting a design review. If code is not user-facing, delete design reviewer checklist
Verify new pages have been added to .pa11yci file so that they will be tested with our automated accessibility testing
Checked keyboard navigability
Tested general usability, landmarks, page header structure, and links with a screen reader (such as Voiceover or ANDI)

As a code reviewer, I have

Reviewed, tested, and left feedback about the changes

Pulled this branch locally and tested it
Verified code meets all checks above. Address any checks that are not satisfied
Reviewed this code and left comments. Indicate if comments must be addressed before code is merged
Checked that all code is adequately covered by tests
Verify migrations are valid and do not conflict with existing migrations

Validated user-facing changes as a developer

Note: Multiple code reviewers can share the checklists above, a second reviewer should not make a duplicate checklist. All checks should be checked before approving, even those labeled N/A.

New pages have been added to .pa11yci file so that they will be tested with our automated accessibility testing
Checked keyboard navigability
Meets all designs and user flows provided by design/product
Tested general usability, landmarks, page header structure, and links with a screen reader (such as Voiceover or ANDI)
(Rarely needed) Tested as both an analyst and applicant user

As a designer reviewer, I have

Verified that the changes match the design intention

Checked that the design translated visually
Checked behavior. Comment any found issues or broken flows.
Checked different states (empty, one, some, error)
Checked for landmarks, page heading structure, and links

Validated user-facing changes as a designer

Checked keyboard navigability
Tested general usability, landmarks, page header structure, and links with a screen reader (such as Voiceover or ANDI)
Tested with multiple browsers (check off which ones were used)
- Chrome
- Microsoft Edge
- FireFox
- Safari
(Rarely needed) Tested as both an analyst and applicant user

References

Code review best practices

Screenshots

github-actions · 2025-09-16T18:04:08Z

🥳 Successfully deployed to developer sandbox kma.

github-actions · 2025-09-16T19:49:07Z

🥳 Successfully deployed to developer sandbox kma.

github-actions · 2025-09-16T20:45:15Z

🥳 Successfully deployed to developer sandbox kma.

asaki222 · 2025-09-18T19:16:41Z

src/registrar/views/domain.py

+                        self.dns_host_service.register_nameservers(zone_name, nameservers)
+                    except (RegistryError, RegistrySystemError, Exception) as e:
+                        logger.error(f"Error updating registry: {e}")



I might not have the full context here, but why was this error message updated?

For some reason, I was hitting an error that wasn't the original one (RegistrySystemError), so then it wasn't getting caught and the app was crashing. So I added other exceptions to catch it (at it's base it would at least be an Exception). I think error handling has a lot to be desired and I'm hoping to focus on a proper and comprehensive error handling pattern in a future ticket.

asaki222 · 2025-09-18T19:19:33Z

src/registrar/views/domain.py

        self.dns_record = None
+        self.client = Client()
+        self.dns_host_service = DnsHostService(client=self.client)



This is awesome! It makes more flexible for other instances :-D

asaki222 · 2025-09-18T19:34:50Z

src/registrar/tests/services/test_cloudflare_service.py

+        },
+        {"test_name": "RequestError", "error": {"exception": RequestError, "message": "Unknown error"}},
+    ]



I like how you organized the the failure cases here

github-actions · 2025-09-18T22:28:17Z

🥳 Successfully deployed to developer sandbox kma.

github-actions · 2025-09-22T16:48:39Z

🥳 Successfully deployed to developer sandbox kma.

github-actions · 2025-09-22T21:40:30Z

🥳 Successfully deployed to developer sandbox kma.

github-actions · 2025-09-22T22:36:00Z

🥳 Successfully deployed to developer sandbox kma.

github-actions · 2025-09-23T22:49:36Z

🥳 Successfully deployed to developer sandbox kma.

github-actions · 2025-09-23T22:57:25Z

🥳 Successfully deployed to developer sandbox kma.

…ates

github-actions · 2025-10-06T17:18:26Z

🥳 Successfully deployed to developer sandbox kma.

github-actions · 2025-10-06T17:42:13Z

🥳 Successfully deployed to developer sandbox kma.

kimallen · 2025-10-06T19:14:13Z

src/registrar/tests/test_views_domain.py

-            _ = UserDomainRole(user=self.user, domain=domain_contact, role="admin").save()
-            page = self.client.get(reverse("domain-security-email", kwargs={"domain_pk": domain_contact.id}))
+            _ = UserDomainRole(user=self.user, domain=domain, role="admin").save()
+            DomainInformation.objects.get_or_create(requester=self.user, domain=domain)


This test wasn't actually testing what it was supposed to. The page wasn't actually rendering the domain because it was missing the data was not set up properly. Adding DomainInformation sets it up properly.

kimallen · 2025-10-06T19:16:51Z

src/registrar/tests/test_admin.py

-            self.assertContains(response, "invited", count=5)
-            self.assertContains(response, "Invited", count=2)
-            self.assertContains(response, "retrieved", count=4)
+            self.assertContains(response, "invited", count=4)


These and the other adjusted counts in this PR are changed because one of the libraries that got updated changed the way the response html shows up. Previous, a lot more cruft (including queries) were showing up. With the updated, there's a lot less included in the response.

The tests in general need some work. They are not very clear what data is created or what data should be expected. For now they are basically serving as a "snapshot", so if something changes, we'll know. In the future, we should adjust the tests to be more effective and focused.

github-actions · 2025-10-06T19:23:38Z

🥳 Successfully deployed to developer sandbox kma.

github-actions · 2025-10-06T19:35:44Z

🥳 Successfully deployed to developer sandbox kma.

* Add httpx library * Update create_account and create_zone to use httpx library * Fix nameserver call and exceptions * Update create_dns_record to use httpx * Update create_account tests * Update create_zone tests * Update create_dns_record tests * Update fetches and tests * Remove context manager and explicitly close client * Update cloudflare_service tests to use new structure with client * Update dns_host_service tests to pass in client * Remove unused helper fn and fix black formatting * Remove test domains from valid_domains list * Remove import of non-exisistent module * Refactor tests and include RequestError tests * Fix linting issues * Mock env't vars in test to pass CI * Move vars into class vars; update test * Fix linting issues * Remove unused import * Update tests to reflect new output of response html after library updates * Update more tests * Fix additional tests * More fixes

github-project-automation bot added this to .gov Product Board Sep 16, 2025

kimallen added 13 commits September 16, 2025 10:51

Add httpx library

12b6109

Update create_account and create_zone to use httpx library

c36d5b3

Fix nameserver call and exceptions

4d61e7c

Update create_dns_record to use httpx

ce0eed0

Update create_account tests

dfc9e14

Update create_zone tests

ad65187

Update create_dns_record tests

8657ff1

Update fetches and tests

97677fa

Remove context manager and explicitly close client

7958a91

Update cloudflare_service tests to use new structure with client

4b9843e

Update dns_host_service tests to pass in client

ef1bffc

Remove unused helper fn and fix black formatting

4a58cc7

Remove test domains from valid_domains list

3b9bcd4

kimallen force-pushed the kma/4152-use-httpx-for-dns-hosting branch from 4f2c3ef to 3b9bcd4 Compare September 16, 2025 17:52

kimallen requested a review from a team September 16, 2025 17:56

kimallen added the Feature: 🌐 DNS hosting Set some RRs in a .gov domain's zone label Sep 16, 2025

Remove import of non-exisistent module

ac098bd

Refactor tests and include RequestError tests

8a7ba58

Fix linting issues

8d341e4

asaki222 self-assigned this Sep 17, 2025

asaki222 reviewed Sep 18, 2025

View reviewed changes

Merge branch 'main' into kma/4152-use-httpx-for-dns-hosting

0cb4149

Merge branch 'main' into kma/4152-use-httpx-for-dns-hosting

3013d4f

asaki222 approved these changes Sep 22, 2025

View reviewed changes

Mock env't vars in test to pass CI

fae4879

Move vars into class vars; update test

643e813

kimallen force-pushed the kma/4152-use-httpx-for-dns-hosting branch from 6f46df6 to 643e813 Compare September 23, 2025 22:45

kimallen added 3 commits September 23, 2025 15:51

Fix linting issues

fd265d4

Remove unused import

7a8b99a

Merge branch 'main' into kma/4152-use-httpx-for-dns-hosting

5834864

kimallen added 3 commits October 1, 2025 15:10

Merge branch 'main' into kma/4152-use-httpx-for-dns-hosting

3207bf7

Merge branch 'main' into kma/4152-use-httpx-for-dns-hosting

79d0591

Update tests to reflect new output of response html after library upd…

238133b

…ates

Update more tests

2da210f

Fix additional tests

797f080

kimallen commented Oct 6, 2025

View reviewed changes

More fixes

aa5af0f

kimallen merged commit 974d759 into main Oct 6, 2025
13 of 14 checks passed

kimallen deleted the kma/4152-use-httpx-for-dns-hosting branch October 6, 2025 19:45

github-project-automation bot moved this to ✅ Done in .gov Product Board Oct 6, 2025

#4152: Use httpx as the api library for dns hosting #4187

#4152: Use httpx as the api library for dns hosting #4187

Uh oh!

Conversation

kimallen commented Sep 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Ticket

Changes

Context for reviewers

Setup

Code Review Verification Steps

As the original developer, I have

Satisfied acceptance criteria and met development standards

Ensured code standards are met (Original Developer)

Validated user-facing changes (if applicable)

As a code reviewer, I have

Reviewed, tested, and left feedback about the changes

Validated user-facing changes as a developer

As a designer reviewer, I have

Verified that the changes match the design intention

Validated user-facing changes as a designer

References

Screenshots

Uh oh!

github-actions bot commented Sep 16, 2025

Uh oh!

github-actions bot commented Sep 16, 2025

Uh oh!

github-actions bot commented Sep 16, 2025

Uh oh!

asaki222 Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

kimallen Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

asaki222 Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

asaki222 Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Sep 18, 2025

Uh oh!

github-actions bot commented Sep 22, 2025

Uh oh!

github-actions bot commented Sep 22, 2025

Uh oh!

github-actions bot commented Sep 22, 2025

Uh oh!

github-actions bot commented Sep 23, 2025

Uh oh!

github-actions bot commented Sep 23, 2025

Uh oh!

github-actions bot commented Oct 6, 2025

Uh oh!

github-actions bot commented Oct 6, 2025

Uh oh!

kimallen Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

kimallen Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Oct 6, 2025

Uh oh!

github-actions bot commented Oct 6, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kimallen commented Sep 16, 2025 •

edited

Loading