Repository: certbot/certbot
Pull request: Add async interface for finalization to acme.client.ClientV2
Add `begin_order_finalization()`/`poll_finalization()` to `acme.client.ClientV2`, which are directly analogous to `answer_challenge()`/`poll_authorizations()`. This allows us to finalize an order and then later poll for its completion as separate steps. Also rewrite `finalize_order()` to call these two methods (which are just the previous body of `finalize_order()` split in two.)

I've not added new test cases because the existing test cases for `finalize_order()` exercise all of the same code paths, but I can if we want to belt-and-suspenders this.

Fixes #9620 

## Pull Request Checklist

- [x] The Certbot team has recently expressed interest in reviewing a PR for this. If not, this PR may be closed due our limited resources and need to prioritize how we spend them.
- [x] If the change being made is to a [distributed component](https://certbot.eff.org/docs/contributing.html#code-components-and-layout), edit the `master` section of `certbot/CHANGELOG.md` to include a description of the change being made.
- [x] Add or update any documentation as needed to support the changes in this PR.
- [x] Include your name in `AUTHORS.md` if you like.


Problem statement:
acme: no interface for asynchronous finalization a la validation
`acme` provides API for answering challenges and later polling the status of authorizations. It would be very helpful for our use case if similar API existed for finalization (separating "start finalization" from "poll for completion of finalization"), which would essentially just entail splitting `acme.client.ClientV2.finalize_order` into two methods (one for starting finalization and one for polling for status) that `finalize_order` would call (to avoid breaking API). I'm more than happy to do the refactor myself if there's appetite for a patch doing this; is this something the project would review and take?

Hints from issue discussion:
👍 This sounds like a good idea and I'd be happy to review it.

Commit message: Add async interface for finalization to acme.client.ClientV2

Add `begin_order_finalization()`/`poll_finalization()` to
`acme.client.ClientV2`, which are directly analogous to
`answer_challenge()`/`poll_authorizations()`. This allows us to
finalize an order and then later poll for its completion as separate
steps.

Patch to review:
diff --git a/acme/acme/client.py b/acme/acme/client.py
index ee31f58a79a..ce57ba74fa7 100644
--- a/acme/acme/client.py
+++ b/acme/acme/client.py
@@ -210,23 +210,35 @@ def poll_authorizations(self, orderr: messages.OrderResource, deadline: datetime
             raise errors.ValidationError(failed)
         return orderr.update(authorizations=responses)
 
-    def finalize_order(self, orderr: messages.OrderResource, deadline: datetime.datetime,
-                       fetch_alternative_chains: bool = False) -> messages.OrderResource:
-        """Finalize an order and obtain a certificate.
+    def begin_order_finalization(self, orderr: messages.OrderResource
+                                 ) -> messages.OrderResource:
+        """Start the process of finalizing an order.
 
         :param messages.OrderResource orderr: order to finalize
         :param datetime.datetime deadline: when to stop polling and timeout
-        :param bool fetch_alternative_chains: whether to also fetch alternative
-            certificate chains
 
-        :returns: finalized order
+        :returns: updated order
         :rtype: messages.OrderResource
-
         """
         csr = OpenSSL.crypto.load_certificate_request(
             OpenSSL.crypto.FILETYPE_PEM, orderr.csr_pem)
         wrapped_csr = messages.CertificateRequest(csr=jose.ComparableX509(csr))
-        self._post(orderr.body.finalize, wrapped_csr)
+        res = self._post(orderr.body.finalize, wrapped_csr)
+        orderr = orderr.update(body=messages.Order.from_json(res.json()))
+        return orderr
+
+    def poll_finalization(self, orderr: messages.OrderResource,
+                          deadline: datetime.datetime,
+                          fetch_alternative_chains: bool = False
+                          ) -> messages.OrderResource:
+        """
+        Poll an order that has been finalized for its status.
+        If it becomes valid, obtain the certificate.
+
+        :returns: finalized order (with certificate)
+        :rtype: messages.OrderResource
+        """
+
         while datetime.datetime.now() < deadline:
             time.sleep(1)
             response = self._post_as_get(orderr.uri)
@@ -247,6 +259,22 @@ def finalize_order(self, orderr: messages.OrderResource, deadline: datetime.date
                 return orderr
         raise errors.TimeoutError()
 
+    def finalize_order(self, orderr: messages.OrderResource, deadline: datetime.datetime,
+                       fetch_alternative_chains: bool = False) -> messages.OrderResource:
+        """Finalize an order and obtain a certificate.
+
+        :param messages.OrderResource orderr: order to finalize
+        :param datetime.datetime deadline: when to stop polling and timeout
+        :param bool fetch_alternative_chains: whether to also fetch alternative
+            certificate chains
+
+        :returns: finalized order
+        :rtype: messages.OrderResource
+
+        """
+        self.begin_order_finalization(orderr)
+        return self.poll_finalization(orderr, deadline, fetch_alternative_chains)
+
     def revoke(self, cert: jose.ComparableX509, rsn: int) -> None:
         """Revoke certificate.
 
diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md
index 6d7e46715d1..f0c755b2acb 100644
--- a/certbot/CHANGELOG.md
+++ b/certbot/CHANGELOG.md
@@ -8,6 +8,9 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
 
 * `acme.messages.OrderResource` now supports being round-tripped
   through JSON
+* `acme.client.ClientV2` now provides an asynchronous interface for
+  order finalization (`begin_order_finalization()` and
+  `poll_finalization()`)
 
 ### Changed
 


Code review comments:
diff_hunk: @@ -210,23 +210,35 @@ def poll_authorizations(self, orderr: messages.OrderResource, deadline: datetime
             raise errors.ValidationError(failed)
         return orderr.update(authorizations=responses)
 
-    def finalize_order(self, orderr: messages.OrderResource, deadline: datetime.datetime,
-                       fetch_alternative_chains: bool = False) -> messages.OrderResource:
-        """Finalize an order and obtain a certificate.
+    def begin_order_finalization(self, orderr: messages.OrderResource
path: acme/acme/client.py
text: @user1:
It would be good to align this name either with  `poll_finalization` or `finalize_order`. Perhaps `submit_finalization` or `begin_finalization`.