Address review comments

certbot-apache/certbot_apache/_internal/prefetch_ocsp.py

@@ -277,6 +277,8 @@ class OCSPPrefetchMixin(object):
         message if unable to copy, but does not error out as it would
         prevent other critical functions that need to be carried out for
         Apache httpd.
+
+        Erroring out here would prevent any restarts done by Apache plugin.
         """
         self._ensure_ocsp_dirs()
         cache_path = os.path.join(self.config.work_dir, "ocsp", "ocsp_cache.db")
@@ -307,10 +309,18 @@ class OCSPPrefetchMixin(object):
         In OCSP, each client (e.g. browser) would have to query the
         OCSP Responder to validate that the site certificate was not revoked.
 
-        Enabling OCSP Stapling, would allow the web-server to query the OCSP
+        Enabling OCSP Stapling would allow the web-server to query the OCSP
         Responder, and staple its response to the offered certificate during
         TLS. i.e. clients would not have to query the OCSP responder.
 
+        OCSP prefetching functionality addresses some of the pain points in
+        the implementation that's currently preset in Apache httpd. The
+        mitigation provided by Certbot are:
+          * OCSP staples get backed up before, and restored after httpd restart
+          * Valid OCSP staples do not get overwritten with errors in case of
+            network connectivity or OCSP responder issues
+          * The staples get updated asynchronically in the background instead
+            of blocking a incoming request.
         """
 
         # Fail early if we are not able to support this

certbot/certbot/crypto_util.py

@@ -237,8 +237,7 @@ def load_cert(cert_path):
 
 
 def cert_sha1_fingerprint(cert_path):
-    """Read fingerprint of a certificate pointed by its file path
-    and returns sha1 digest of said fingerprint.
+    """Read a certificate by its file path and return its SHA-1 fingerprint.
 
     :param str cert_path: File path to the x509 certificate file
 

certbot/certbot/ocsp.py

@@ -78,7 +78,7 @@ class RevocationChecker(object):
 
         :param str cert_path: Certificate path
         :param str chain_path: Certificate chain filepath
-        :param str response_file: File path to a file containing a raw OCSP response.
+        :param str response_file: File path where the raw OCSP response should be written
 
         :returns: True if revoked; False if valid or the check failed or cert is expired.
         :rtype: bool
@@ -194,9 +194,9 @@ def _ocsp_times_openssl_bin(response_file):
         return None, None, None
 
     prod_str, this_str, next_str = _translate_ocsp_response_times(output)
-    prod_dt = util.parse_datetime(prod_str)
-    this_dt = util.parse_datetime(this_str)
-    next_dt = util.parse_datetime(next_str)
+    prod_dt = _parse_datetime(prod_str)
+    this_dt = _parse_datetime(this_str)
+    next_dt = _parse_datetime(next_str)
     return prod_dt, this_dt, next_dt
 
 
@@ -418,3 +418,19 @@ def _translate_ocsp_response_times(response):
         next_date = next_match.group(1)
 
     return prod_date, this_date, next_date
+
+
+def _parse_datetime(dt_string):
+    """
+    Parses a string to datetime, ignoring timezone.
+
+    :param str dt_string: String representation of date and time
+
+    :returns: datetime representation of time
+    :rtype: datetime.datetime or None
+    """
+    try:
+        dateformat = "%b %d %H:%M:%S %Y %Z"
+        return datetime.strptime(dt_string, dateformat)
+    except ValueError:
+        return None

certbot/certbot/plugins/enhancements.py

@@ -184,7 +184,7 @@ class OCSPPrefetchEnhancement(object):
         needed and if valid, store it to be served for connecting clients.
 
         :param lineage: Certificate lineage object
-        :type lineage: certbot.storage.RenewableCert
+        :type lineage: certbot.interfaces.RenewableCert
 
         .. note:: prepare() method inherited from `interfaces.IPlugin` might need
             to be called manually within implementation of this interface method
@@ -199,7 +199,7 @@ class OCSPPrefetchEnhancement(object):
         over the subsequent runs of Certbot renew.
 
         :param lineage: Certificate lineage object
-        :type lineage: certbot.storage.RenewableCert
+        :type lineage: certbot.interfaces.RenewableCert
         :param domains: List of domains in certificate to enhance
         :type domains: `list` of `str`
         """

certbot/certbot/util.py

@@ -599,19 +599,3 @@ def atexit_register(func, *args, **kwargs):
 def _atexit_call(func, *args, **kwargs):
     if _INITIAL_PID == os.getpid():
         func(*args, **kwargs)
-
-
-def parse_datetime(dt_string):
-    """
-    Parses a string to datetime, ignoring timezone.
-
-    :param str dt_string: String representation of date and time
-
-    :returns: datetime representation of time
-    :rtype: datetime.datetime or None
-    """
-    try:
-        dateformat = "%b %d %H:%M:%S %Y %Z"
-        return datetime.strptime(dt_string, dateformat)
-    except ValueError:
-        return None