openstack/placement
Code Issues Proposed changes

pre-commit: Add sphinx-lint

Browse Source 
Change-Id: I9d9d8b372b0a9c761b605188b3c641c9d1f326cb
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
This commit is contained in:
Stephen Finucane
2024-05-14 14:32:43 +01:00
parent aa520fec9c
commit 0736c61056
8 changed files with 53 additions and 54 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
.pre-commit-config.yaml
View File

@@ -31,11 +31,10 @@ repos:
hooks: hooks:
- id: autopep8 - id: autopep8
files: '^.*\.py$' files: '^.*\.py$'
# TODO(stephenfin): Uncomment this - repo: https://github.com/sphinx-contrib/sphinx-lint
# - repo: https://github.com/sphinx-contrib/sphinx-lint rev: v0.9.1
# rev: v0.9.1 hooks:
# hooks: - id: sphinx-lint
# - id: sphinx-lint args: [--enable=default-role]
# args: [--enable=default-role] files: ^doc/|releasenotes|api-guide
# files: ^doc/|releasenotes|api-guide types: [rst]
# types: [rst]

68
doc/source/contributor/architecture.rst
View File

@@ -29,7 +29,7 @@ persistence layer. In the Stein release, that interface was refactored to
remove the use of versioned objects and split functionality into smaller remove the use of versioned objects and split functionality into smaller
modules. modules.
Though the placement service does not aspire to be a `microservice` it does Though the placement service does not aspire to be a *microservice* it does
aspire to continue to be small and minimally complex. This means a relatively aspire to continue to be small and minimally complex. This means a relatively
small amount of middleware that is not configurable, and a limited number of small amount of middleware that is not configurable, and a limited number of
exposed resources where any given resource is represented by one (and only exposed resources where any given resource is represented by one (and only
@@ -53,16 +53,16 @@ near the surface. The goal of this is to make things easy to trace when
debugging or adding functionality. debugging or adding functionality.
Functionality which is required for every request is handled in raw WSGI Functionality which is required for every request is handled in raw WSGI
middleware that is composed in the `placement.deploy` module. Dispatch or middleware that is composed in the ``placement.deploy`` module. Dispatch or
routing is handled declaratively via the ``ROUTE_DECLARATIONS`` map defined in routing is handled declaratively via the ``ROUTE_DECLARATIONS`` map defined in
the `placement.handler` module. the ``placement.handler`` module.
Mapping is by URL plus request method. The destination is a complete WSGI Mapping is by URL plus request method. The destination is a complete WSGI
application, using a subclass of the `wsgify`_ method from `WebOb`_ to provide application, using a subclass of the `wsgify`_ method from `WebOb`_ to provide
a `Request`_ object that provides convenience methods for accessing request a `Request`_ object that provides convenience methods for accessing request
headers, bodies, and query parameters and for generating responses. In the headers, bodies, and query parameters and for generating responses. In the
placement API these mini-applications are called `handlers`. The `wsgify` placement API these mini-applications are called *handlers*. The ``wsgify``
subclass is provided in `placement.wsgi_wrapper` as `PlacementWsgify`. It is subclass is provided in ``placement.wsgi_wrapper`` as ``PlacementWsgify``. It is
used to make sure that JSON formatted error responses are structured according used to make sure that JSON formatted error responses are structured according
to the API-SIG `errors`_ guideline. to the API-SIG `errors`_ guideline.
@@ -133,7 +133,7 @@ lower microversion should return a ``404``. When adding a new method to an
existing URL a request for a lower microversion should return a ``405``. existing URL a request for a lower microversion should return a ``405``.
In either case, the ``ROUTE_DECLARATIONS`` dictionary in the In either case, the ``ROUTE_DECLARATIONS`` dictionary in the
`placement.handler` module should be updated to point to a ``placement.handler`` module should be updated to point to a
function within a module that contains handlers for the type of entity function within a module that contains handlers for the type of entity
identified by the URL. Collection and individual entity handlers of the same identified by the URL. Collection and individual entity handlers of the same
type should be in the same module. type should be in the same module.
@@ -145,12 +145,12 @@ WebOb `Request`_ object, and return a WebOb `Response`_.
For ``PUT`` and ``POST`` methods, request bodies are expected to be JSON For ``PUT`` and ``POST`` methods, request bodies are expected to be JSON
based on a content-type of ``application/json``. This may be enforced by using based on a content-type of ``application/json``. This may be enforced by using
a decorator: ``@util.require_content('application/json')``. If the body is not a decorator: ``@util.require_content('application/json')``. If the body is not
`JSON`, a ``415`` response status is returned. JSON, a ``415`` response status is returned.
Response bodies are usually `JSON`. A handler can check the `Accept` header Response bodies are usually JSON. A handler can check the ``Accept`` header
provided in a request using another decorator: provided in a request using another decorator:
``@util.check_accept('application/json')``. If the header does not allow ``@util.check_accept('application/json')``. If the header does not allow
`JSON`, a ``406`` response status is returned. JSON, a ``406`` response status is returned.
If a handler returns a response body, a ``Last-Modified`` header should be If a handler returns a response body, a ``Last-Modified`` header should be
included with the response. If the entity or entities in the response body included with the response. If the entity or entities in the response body
@@ -176,21 +176,21 @@ If a ``Last-Modified`` header is set, then a ``Cache-Control`` header with a
value of ``no-cache`` must be set as well. This is to avoid user-agents value of ``no-cache`` must be set as well. This is to avoid user-agents
inadvertently caching the responses. inadvertently caching the responses.
`JSON` sent in a request should be validated against a JSON Schema. A JSON sent in a request should be validated against a JSON Schema. A
``util.extract_json`` method is available. This takes a request body and a ``util.extract_json`` method is available. This takes a request body and a
schema. If multiple schema are used for different microversions of the same schema. If multiple schema are used for different microversions of the same
request, the caller is responsible for selecting the right one before calling request, the caller is responsible for selecting the right one before calling
``extract_json``. ``extract_json``.
When a handler needs to read or write the data store it should use methods on When a handler needs to read or write the data store it should use methods on
the objects found in the `placement.objects` package. Doing so requires a the objects found in the ``placement.objects`` package. Doing so requires a
context which is provided to the handler method via the WSGI environment. It context which is provided to the handler method via the WSGI environment. It
can be retrieved as follows:: can be retrieved as follows::
context = req.environ['placement.context'] context = req.environ['placement.context']
.. note:: If your change requires new methods or new objects in the .. note:: If your change requires new methods or new objects in the
`placement.objects` package, after you have made sure that you really ``placement.objects`` package, after you have made sure that you really
do need those new methods or objects (you may not!) make those do need those new methods or objects (you may not!) make those
changes in a patch that is separate from and prior to the HTTP API changes in a patch that is separate from and prior to the HTTP API
change. change.
@@ -222,11 +222,11 @@ At some point in every application's life it becomes necessary to change the
structure of its database. Modifying the SQLAlchemy models (in structure of its database. Modifying the SQLAlchemy models (in
placement/db/sqlachemy/models.py) is necessary for the application to placement/db/sqlachemy/models.py) is necessary for the application to
understand the new structure, but that will not change the actual underlying understand the new structure, but that will not change the actual underlying
database. To do that, Placement uses `alembic` to run database migrations. database. To do that, Placement uses ``alembic`` to run database migrations.
Alembic calls each change a **revision**. To create a migration with alembic, Alembic calls each change a **revision**. To create a migration with alembic,
run the `alembic revision` command. Alembic will then generate a new revision run the ``alembic revision`` command. Alembic will then generate a new revision
file with a unique file name, and place it in the `alembic/versions/` file with a unique file name, and place it in the ``alembic/versions/``
directory: directory:
.. code-block:: console .. code-block:: console
@@ -271,11 +271,11 @@ The generated file will look something like this:
The top of the file is the docstring that will show when you review your The top of the file is the docstring that will show when you review your
revision history. If we did not include the **-m** comment when we ran the revision history. If we did not include the **-m** comment when we ran the
`alembic revision` command, this would just contain "empty message". If you did ``alembic revision`` command, this would just contain "empty message". If you did
not specify the comment when creating the file, be sure to replace "empty not specify the comment when creating the file, be sure to replace "empty
message" with a brief comment describing the reason for the database change. message" with a brief comment describing the reason for the database change.
You then need to define the changes in the `upgrade()` method. The code used in You then need to define the changes in the ``upgrade()`` method. The code used in
these methods is basic SQLAlchemy code for creating and modifying tables. You these methods is basic SQLAlchemy code for creating and modifying tables. You
can examine existing migrations in the project to see examples of what this can examine existing migrations in the project to see examples of what this
code looks like, as well as find more in-depth usage of Alembic in the `Alembic code looks like, as well as find more in-depth usage of Alembic in the `Alembic
@@ -284,7 +284,7 @@ tutorial`_.
One other option when creating the revision is to add the ``--autogenerate`` One other option when creating the revision is to add the ``--autogenerate``
parameter to the revision command. This assumes that you have already updated parameter to the revision command. This assumes that you have already updated
the SQLAlchemy models, and have a connection to the placement database the SQLAlchemy models, and have a connection to the placement database
configured. When run with this option, the `upgrade()` method of the revision configured. When run with this option, the ``upgrade()`` method of the revision
file is filled in for you by alembic as it compares the schema described in file is filled in for you by alembic as it compares the schema described in
your models.py script and the actual state of the database. You should always your models.py script and the actual state of the database. You should always
verify the revision script to make sure it does just what you intended, both by verify the revision script to make sure it does just what you intended, both by
@@ -299,27 +299,27 @@ This section tries to shed some light on some of the differences between the
placement API and some of the other OpenStack APIs or on situations which may placement API and some of the other OpenStack APIs or on situations which may
be surprising or unexpected. be surprising or unexpected.
* The placement API is somewhat more strict about `Content-Type` and `Accept` * The placement API is somewhat more strict about ``Content-Type`` and ``Accept``
headers in an effort to follow the HTTP RFCs. headers in an effort to follow the HTTP RFCs.
If a user-agent sends some JSON in a `PUT` or `POST` request without a If a user-agent sends some JSON in a ``PUT`` or ``POST`` request without a
`Content-Type` of `application/json` the request will result in an error. ``Content-Type`` of ``application/json`` the request will result in an error.
If a `GET` request is made without an `Accept` header, the response will If a ``GET`` request is made without an ``Accept`` header, the response will
default to being `application/json`. default to being ``application/json``.
If a request is made with an explicit `Accept` header that does not include If a request is made with an explicit ``Accept`` header that does not include
`application/json` then there will be an error and the error will attempt to ``application/json`` then there will be an error and the error will attempt to
be in the requested format (for example, `text/plain`). be in the requested format (for example, ``text/plain``).
* If a URL exists, but a request is made using a method that that URL does not * If a URL exists, but a request is made using a method that that URL does not
support, the API will respond with a `405` error. Sometimes in the nova APIs support, the API will respond with a ``405`` error. Sometimes in the nova APIs
this can be a `404` (which is wrong, but understandable given the constraints this can be a ``404`` (which is wrong, but understandable given the constraints
of the code). of the code).
* Because each handler is individually wrapped by the `PlacementWsgify` * Because each handler is individually wrapped by the ``PlacementWsgify``
decorator any exception that is a subclass of `webob.exc.WSGIHTTPException` decorator any exception that is a subclass of ``webob.exc.WSGIHTTPException``
that is raised from within the handler, such as `webob.exc.HTTPBadRequest`, that is raised from within the handler, such as ``webob.exc.HTTPBadRequest``,
will be caught by WebOb and turned into a valid `Response`_ containing will be caught by WebOb and turned into a valid `Response`_ containing
headers and body set by WebOb based on the information given when the headers and body set by WebOb based on the information given when the
exception was raised. It will not be seen as an exception by any of the exception was raised. It will not be seen as an exception by any of the
@@ -329,9 +329,9 @@ be surprising or unexpected.
example, you are trying to add some middleware that operates on exceptions. example, you are trying to add some middleware that operates on exceptions.
Other exceptions that are not from `WebOb`_ will raise outside the handlers Other exceptions that are not from `WebOb`_ will raise outside the handlers
where they will either be caught in the `__call__` method of the where they will either be caught in the ``__call__`` method of the
`PlacementHandler` app that is responsible for dispatch, or by the ``PlacementHandler`` app that is responsible for dispatch, or by the
`FaultWrap` middleware. ``FaultWrap`` middleware.
.. _WSGI: https://www.python.org/dev/peps/pep-3333/ .. _WSGI: https://www.python.org/dev/peps/pep-3333/

2
doc/source/contributor/goals.rst
View File

@@ -34,7 +34,7 @@ Don't Use Global Config
----------------------- -----------------------
Placement uses `oslo.config`_ to manage configuration, passing a reference to Placement uses `oslo.config`_ to manage configuration, passing a reference to
an `oslo_config.cfg.ConfigOpts` as required. Before things `were changed`_ a an ``oslo_config.cfg.ConfigOpts`` as required. Before things `were changed`_ a
global was used instead. Placement inherited this behavior from nova, where global was used instead. Placement inherited this behavior from nova, where
using a global ``CONF`` is the normal way to interact with the configuration using a global ``CONF`` is the normal way to interact with the configuration
options. Continuing this pattern in placement made it difficult for nova to use options. Continuing this pattern in placement made it difficult for nova to use

2
doc/source/contributor/testing.rst
View File

@@ -16,7 +16,7 @@
=================== ===================
Most of the handler code in the placement API is tested using `gabbi`_. Some Most of the handler code in the placement API is tested using `gabbi`_. Some
utility code is tested with unit tests found in `placement/tests/unit`. The utility code is tested with unit tests found in ``placement/tests/unit``. The
back-end objects are tested with a combination of unit and functional tests back-end objects are tested with a combination of unit and functional tests
found in ``placement/tests/unit/objects`` and found in ``placement/tests/unit/objects`` and
``placement/tests/functional/db``. ``placement/tests/functional/db``.

4
doc/source/specs/train/implemented/2005297-negative-aggregate-membership.rst
View File

@@ -43,11 +43,11 @@ This feature is useful to save special resources for specific users.
Use Case 1 Use Case 1
~~~~~~~~~~ ~~~~~~~~~~
Some of the compute host are `Licensed Windows Compute Host`, meaning any VMs Some of the compute host are *Licensed Windows Compute Host*, meaning any VMs
booted on this compute host will be considered as licensed Windows image and booted on this compute host will be considered as licensed Windows image and
depending on the usage of VM, operator will charge it to the end-users. depending on the usage of VM, operator will charge it to the end-users.
As an operator, I want to avoid booting images/volumes other than Windows OS As an operator, I want to avoid booting images/volumes other than Windows OS
on `Licensed Windows Compute Host`. on *Licensed Windows Compute Host*.
Use Case 2 Use Case 2
~~~~~~~~~~ ~~~~~~~~~~

12
doc/source/specs/xena/implemented/allow-provider-re-parenting.rst
View File

@@ -112,24 +112,24 @@ microversion and extensive documentation to signal the new behavior.
Still there is the list of alternatives discussed during the review: Still there is the list of alternatives discussed during the review:
* `Do nothing`: While it is considered not safe enough during the PTG, during * *Do nothing*: While it is considered not safe enough during the PTG, during
the spec review we ended up choosing this as the main solution. the spec review we ended up choosing this as the main solution.
* `A new query parameter`: A new query parameter is proposed for the * *A new query parameter*: A new query parameter is proposed for the
``PUT /resource_providers/{uuid}`` API called ``allow_reparenting`` the ``PUT /resource_providers/{uuid}`` API called ``allow_reparenting`` the
default value of the query parameter is ``False`` and the re-parenting cases default value of the query parameter is ``False`` and the re-parenting cases
defined in this spec is only accepted by Placement if the request contains defined in this spec is only accepted by Placement if the request contains
the new query parameter with the ``True``. It is considered hacky to add a the new query parameter with the ``True``. It is considered hacky to add a
query parameter for a PUT request. query parameter for a PUT request.
* `A new field in the request body`: This new field would have the same meaning * *A new field in the request body*: This new field would have the same meaning
as the proposed query parameter but it would be put into the request body. It as the proposed query parameter but it would be put into the request body. It
is considered non-RESTful as such field is not persisted or returned as the is considered non-RESTful as such field is not persisted or returned as the
result of the PUT request as it does not belong to the representation of the result of the PUT request as it does not belong to the representation of the
ResourceProvider entity the PUT request updates. ResourceProvider entity the PUT request updates.
* `A new Header`: Instead of a new query paramtere use a new HTTP header * *A new Header*: Instead of a new query paramtere use a new HTTP header
``x-openstack-placement-allow-provider-reparenting:True``. As the name shows ``x-openstack-placement-allow-provider-reparenting:True``. As the name shows
this needs a lot more context encoded in it to be specific for the API it this needs a lot more context encoded in it to be specific for the API it
modifies while the query parameter already totally API specific. modifies while the query parameter already totally API specific.
* `Use a PATCH request for updating the parent`: While this would make the * *Use a PATCH request for updating the parent*: While this would make the
parent change more explicit it would also cause great confusion for the parent change more explicit it would also cause great confusion for the
client for multiple reasons: client for multiple reasons:
@@ -138,7 +138,7 @@ Still there is the list of alternatives discussed during the review:
2) Changing the ``parent_uuid`` field from None to a valid RP uuid is 2) Changing the ``parent_uuid`` field from None to a valid RP uuid is
supported by the PUT request but to change it from one RP uuid to another supported by the PUT request but to change it from one RP uuid to another
would require a totally different ``PATCH`` request. would require a totally different ``PATCH`` request.
* `Use a sub resource`: Signal the explicit re-parenting either in a form of * *Use a sub resource*: Signal the explicit re-parenting either in a form of
``PUT /resource-providers/{uuid}/force`` or ``PUT /resource-providers/{uuid}/force`` or
``PUT /resource-providers/{uuid}/parent_uuid/{parent}``. While the second ``PUT /resource-providers/{uuid}/parent_uuid/{parent}``. While the second
option seems to be acceptable to multiple reviewers, I think it will be option seems to be acceptable to multiple reviewers, I think it will be