From 698645ca5ceaf273b93fc7129553e9e22cec85d1 Mon Sep 17 00:00:00 2001
From: Eric Fried <openstack@fried.cc>
Date: Mon, 17 Jun 2019 10:26:02 -0500
Subject: [PATCH] Miscellaneous doc/comment/log cleanups

This commit is a grab-bag of tweaks and cleanups noticed while working
in the neighborhood on something unrelated.

- Add a descriptive comment to the ResourceProviderNotFound exception.
  This is a marker exception used for inter-method communication.
- Add and enhance debug logs in the in_tree$S filter code path.
- Correct a SQL comment in get_providers_with_resource (missed when the
  code was tweaked).
- Removes a TODO about an optimization that became obsolete when
  RequestGroupSearchContext was integrated into
  get_provider_ids_matching.
- Corrects the get_sharing_providers docstring s/list/set/ to reflect
  reality.
- Changes a link in the provider-tree doc from the spec-in-progress to
  the merged and published version.

Change-Id: Id433b0540e2051ab27b2c22cc14c62105555ea8f
---
 doc/source/usage/provider-tree.rst    |  2 +-
 placement/exception.py                |  5 ++++-
 placement/objects/research_context.py | 17 ++++++++---------
 3 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/doc/source/usage/provider-tree.rst b/doc/source/usage/provider-tree.rst
index dbc385acc..404112c9f 100644
--- a/doc/source/usage/provider-tree.rst
+++ b/doc/source/usage/provider-tree.rst
@@ -504,4 +504,4 @@ each optionally prefixed with ``!`` to indicate that it is forbidden.
 .. _`Granular Resource Request`: https://specs.openstack.org/openstack/nova-specs/specs/rocky/implemented/granular-resource-requests.html
 .. _`Filter Allocation Candidates by Provider Tree`: https://specs.openstack.org/openstack/nova-specs/specs/stein/implemented/alloc-candidates-in-tree.html
 .. _`Support subtree filter`: https://review.opendev.org/#/c/595236/
-.. _`root_required`: https://review.opendev.org/#/c/662191/5/doc/source/specs/train/approved/2005575-nested-magic-1.rst@304
\ No newline at end of file
+.. _`root_required`: https://docs.openstack.org/placement/latest/specs/train/approved/2005575-nested-magic-1.html#root-required
diff --git a/placement/exception.py b/placement/exception.py
index 0b6c15e3a..308c424de 100644
--- a/placement/exception.py
+++ b/placement/exception.py
@@ -84,7 +84,10 @@ class ResourceProviderConcurrentUpdateDetected(ConcurrentUpdateDetected):
 
 
 class ResourceProviderNotFound(NotFound):
-    msg_fmt = "No such resource provider(s)"
+    # Marker exception indicating that we've filtered down to zero possible
+    # allocation candidates. Does not represent an API error; should only be
+    # used internally: no results is a 200 with empty allocation_requests.
+    msg_fmt = "No results are possible."
 
 
 class InvalidAllocationCapacityExceeded(InvalidInventory):
diff --git a/placement/objects/research_context.py b/placement/objects/research_context.py
index fed21b601..ee69c7943 100644
--- a/placement/objects/research_context.py
+++ b/placement/objects/research_context.py
@@ -106,10 +106,13 @@ class RequestGroupSearchContext(object):
         if group.in_tree:
             tree_ids = provider_ids_from_uuid(context, group.in_tree)
             if tree_ids is None:
+                LOG.debug("No provider found for in_tree%s=%s",
+                          suffix, group.in_tree)
                 raise exception.ResourceProviderNotFound()
             self.tree_root_id = tree_ids.root_id
-            LOG.debug("getting allocation candidates in the same tree "
-                      "with the root provider %s", tree_ids.root_uuid)
+            LOG.debug("Group %s getting allocation candidates in the same "
+                      "tree with the root provider %s",
+                      self.suffix, tree_ids.root_uuid)
 
         self._rps_with_resource = {}
         for rc_id, amount in self.resources.items():
@@ -435,6 +438,8 @@ def get_providers_with_resource(ctx, rc_id, amount, tree_root_id=None):
     #  AND inv.min_unit <= $AMOUNT
     #  AND inv.max_unit >= $AMOUNT
     #  AND $AMOUNT % inv.step_size == 0
+    #  # If tree_root_id specified:
+    #  AND rp.root_provider_id == $tree_root_id
     rpt = sa.alias(_RP_TBL, name="rp")
     inv = sa.alias(_INV_TBL, name="inv")
     usage = _usage_select([rc_id])
@@ -490,12 +495,6 @@ def get_provider_ids_matching(rg_ctx):
     # purposes of rough debugging of a single allocation candidates request) as
     # well as reduce the necessary knowledge of SQL in order to understand the
     # queries being executed here.
-    #
-    # NOTE(jaypipes): The efficiency of this operation may be improved by
-    # passing the trait_rps and/or forbidden_ip_ids iterables to the
-    # get_providers_with_resource() function so that we don't have to process
-    # as many records inside the loop below to remove providers from the
-    # eventual results list
     provs_with_resource = set()
     first = True
     for rc_id, amount in rg_ctx.resources.items():
@@ -1049,7 +1048,7 @@ def get_provider_ids_for_traits_and_aggs(rg_ctx):
 
 @db_api.placement_context_manager.reader
 def get_sharing_providers(ctx, rp_ids=None):
-    """Returns a list of resource provider IDs (internal IDs, not UUIDs)
+    """Returns a set of resource provider IDs (internal IDs, not UUIDs)
     that indicate that they share resource via an aggregate association.
 
     Shared resource providers are marked with a standard trait called