Skip to content

Commit

Permalink
DRIVERS-2790 Add more linting and integrity checks (#1464)
Browse files Browse the repository at this point in the history
* Add pre-commit file

* Add pre-commit and sphinx build check

* undo change
  • Loading branch information
blink1073 authored Dec 6, 2023
1 parent 13117d6 commit fd24a0a
Show file tree
Hide file tree
Showing 15 changed files with 144 additions and 67 deletions.
39 changes: 39 additions & 0 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
name: Lint Files

on:
push:
branches: [master]
pull_request:

jobs:
# Run "pre-commit run --all-files"
pre-commit:
runs-on: ubuntu-latest
timeout-minutes: 5

steps:
- uses: actions/checkout@v4
- uses: actions/setup-python@v4

# ref: https://github.com/pre-commit/action
- uses: pre-commit/action@v3.0.0
- name: Help message if pre-commit fail
if: ${{ failure() }}
run: |
echo "You can install pre-commit hooks to automatically run formatting"
echo "on each commit with:"
echo " pre-commit install"
echo "or you can run by hand on staged files with"
echo " pre-commit run"
echo "or after-the-fact on already committed files with"
echo " pre-commit run --all-files --hook-stage manual"
sphinx-build:
runs-on: ubuntu-latest
timeout-minutes: 5

steps:
- uses: actions/checkout@v4
- uses: actions/setup-python@v4
- run: pip install sphinx
- run: cd source && sphinx-build -W -b text . docs_build index.rst
25 changes: 0 additions & 25 deletions .github/workflows/rstcheck.yml

This file was deleted.

1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ logo.png
.DS_Store
codereview.rc
.idea/**
docs_build
49 changes: 49 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@

repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.4.0
hooks:
- id: check-case-conflict
- id: check-executables-have-shebangs
- id: check-added-large-files
- id: check-case-conflict
- id: check-merge-conflict
#- id: end-of-file-fixer
#- id: trailing-whitespace
#- id: check-json

# We use the Python version instead of the original version which seems to require Docker
# https://github.com/koalaman/shellcheck-precommit
- repo: https://github.com/shellcheck-py/shellcheck-py
rev: v0.9.0.6
hooks:
- id: shellcheck
name: shellcheck
args: ["--severity=error"]
stages: [manual]

- repo: https://github.com/rstcheck/rstcheck
rev: v6.2.0
hooks:
- id: rstcheck
additional_dependencies: [sphinx]
args: ["--ignore-languages=c,python,java", "--report-level=error"]
stages: [manual]

- repo: https://github.com/python-jsonschema/check-jsonschema
rev: 0.26.3
hooks:
- id: check-github-workflows

- repo: https://github.com/pre-commit/pygrep-hooks
rev: "v1.10.0"
hooks:
# - id: rst-backticks
- id: rst-directive-colons
- id: rst-inline-touching-normal

# - repo: https://github.com/codespell-project/codespell
# rev: "v2.2.6"
# hooks:
# - id: codespell
# exclude_types: [json,yaml]
50 changes: 30 additions & 20 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,29 @@ Store all source documents in the ``source/`` directory.

.. _`reStructuredText`: http://docutils.sourceforge.net/rst.html

Linting
-------

This repo uses `pre-commit <https://pypi.org/project/pre-commit/>`_
for managing linting.
``pre-commit`` performs various checks on the files and uses tools
that help follow a consistent style within the repo.

To set up ``pre-commit`` locally, run:

.. code:: bash
pip install pre-commit # or brew install pre-commit
pre-commit install
To run ``pre-commit`` manually, run ``pre-commit run --all-files``.

To run a manual hook like ``rstcheck`` manually, run:

.. code:: bash
pre-commit run --all-files --hook-stage manual rstcheck
Prose test numbering
--------------------

Expand All @@ -125,28 +148,15 @@ by striking through or replacing the entire test with a note (e.g. **Removed**).
Building Documents
------------------

To build documents issue the ``make`` command in a local copy of this
repository. The output PDFs end up in the ``build/`` directory. The
build depends on:

- `Python Docutils <http://pypi.python.org/pypi/docutils>`_

- A functioning basic LaTeX/TeX install with ``pdflatex``. If you run
OS X, use `MacTeX`_

``make all`` will build all documents in the ``source/`` folder. The
system builds all targets in ``build/``.

Run ``make setup`` to generate (or regenerate) a ``makefile.generated``
file which provides specific targets for all files in the source file
so you can choose to compile only some of the files that you
need. Once generated, running "``make [file-name-without-extension]``"
will rebuild only those files (if needed.)
We build the docs in ``text`` mode in CI to make sure they build without errors.
We don't actually support building html, since we rely on GitHub to render the documents.
To build locally, run:

Use ``make clean`` to remove the ``build/`` directory and "``make
cleanup``" to remove the LaTeX by-products from ``build/``.
.. code:: bash
.. _`MacTeX` : http://www.tug.org/mactex/
pip install sphinx
cd source
sphinx-build -W -b text . docs_build index.rst
Converting to JSON
------------------
Expand Down
4 changes: 3 additions & 1 deletion source/auth/tests/mongodb-aws.rst
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ Drivers MUST be able to authentiate using a valid OIDC token and associated
role ARN taken from environment variables, respectively:

.. code-block::
AWS_WEB_IDENTITY_TOKEN_FILE
AWS_ROLE_ARN
AWS_ROLE_SESSION_NAME (optional)
Expand All @@ -82,6 +83,7 @@ A sample URI in for a web identity test would be:
Drivers MUST test with and without AWS_ROLE_SESSION_NAME set.

.. note:: No username, password or session token is passed into the URI.

Drivers MUST check the environment variables listed above and make an `AssumeRoleWithWebIdentity request <https://docs.aws.amazon.com/STS/latest/APIReference/API_AssumeRoleWithWebIdentity.html>`_ to obtain
credentials.

Expand Down Expand Up @@ -166,4 +168,4 @@ with the auth provider directly instead of using a client.
#. Set the AWS environment variables to invalid values.
#. Create a new client.
#. Ensure that a ``find`` operation succeeds.
#. Clear the AWS environment variables.
#. Clear the AWS environment variables.
4 changes: 2 additions & 2 deletions source/client-side-encryption/client-side-encryption.rst
Original file line number Diff line number Diff line change
Expand Up @@ -812,7 +812,7 @@ load. Refer:


``extraOptions.cryptSharedLibRequired``
````````````````````````````````````
```````````````````````````````````````

:type: :ts:`boolean`
:default: |false|
Expand Down Expand Up @@ -1016,7 +1016,7 @@ method and MUST NOT be passed to the `drop`_ command.
and no longer allows names to deviate from the following:

- ``enxcol_.<collectionName>.esc``
- ``enxcol_.<collectionName>.ecoc`
- ``enxcol_.<collectionName>.ecoc``

Drivers SHOULD NOT document the ``escCollection`` and ``ecocCollection``
options.
Expand Down
6 changes: 3 additions & 3 deletions source/client-side-encryption/tests/README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1159,7 +1159,7 @@ Proceed to run the test case.

Each test case configures a ``MongoClient`` with automatic encryption (named ``client_encrypted``).

Each test must assert the number of unique ``MongoClient``s created. This can be accomplished by capturing ``TopologyOpeningEvent``, or by checking command started events for a client identifier (not possible in all drivers).
Each test must assert the number of unique ``MongoClient`` objects created. This can be accomplished by capturing ``TopologyOpeningEvent``, or by checking command started events for a client identifier (not possible in all drivers).

Running a test case
```````````````````
Expand All @@ -1177,7 +1177,7 @@ Running a test case
- Use ``client_encrypted`` to run a ``findOne`` operation on ``db.coll``, with the filter ``{ "_id": 0 }``.
- Expect the result to be ``{ "_id": 0, "encrypted": "string0" }``.
- Check captured events against ``TestCase.Expectations``.
- Check the number of unique ``MongoClient``s created is equal to ``TestCase.ExpectedNumberOfClients``.
- Check the number of unique ``MongoClient`` objects created is equal to ``TestCase.ExpectedNumberOfClients``.

Case 1
``````
Expand Down Expand Up @@ -3056,7 +3056,7 @@ Assert that an error was raised.


Case 8: setting precision errors if the type is not double or Decimal128
````````````````````````````````````````````````````````````````````
````````````````````````````````````````````````````````````````````````
This test case should be skipped if the encrypted field is ``encryptedDoublePrecision``, ``encryptedDoubleNoPrecision``, ``encryptedDecimalPrecision``, or ``encryptedDecimalNoPrecision``.

Use ``clientEncryption.encrypt()`` to encrypt the value 6. Ensure the type matches that of the encrypted field.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ Publishing & Subscribing

The driver SHOULD publish events in a manner that is standard to the driver's language publish/subscribe patterns and is not strictly mandated in this specification.

Similarly, as described in the `logging specification <../logging/logging.rst#implementation-requirements>`_ the driver SHOULD emit log messages in a manner that is standard for the language.
Similarly, as described in the `logging specification <../logging/logging.rst#implementation-requirements>`__ the driver SHOULD emit log messages in a manner that is standard for the language.


Guarantees
Expand Down Expand Up @@ -363,7 +363,7 @@ See the `Load Balancer Specification <../load-balancers/load-balancers.rst#event
Log Messages
------------
Please refer to the `logging specification <../logging/logging.rst>`_ for details on logging implementations in general, including log levels, log
Please refer to the `logging specification <../logging/logging.rst>`__ for details on logging implementations in general, including log levels, log
components, and structured versus unstructured logging.

Drivers MUST support logging of command information via the following types of log messages. These messages MUST be logged at ``Debug`` level and use
Expand Down Expand Up @@ -451,7 +451,7 @@ In addition to the common fields, command started messages MUST contain the foll
* - command
- String
- Relaxed extJSON representation of the command. This document MUST be truncated appropriately according to rules defined in the
`logging specification <../logging/logging.rst#configurable-max-document-length>`_, and MUST be replaced with an empty document
`logging specification <../logging/logging.rst#configurable-max-document-length>`__, and MUST be replaced with an empty document
"{ }" if the command is considered sensitive.

The unstructured form SHOULD be as follows, using the values defined in the structured format above to fill in placeholders as appropriate:
Expand Down Expand Up @@ -484,7 +484,7 @@ In addition to the common fields, command succeeded messages MUST contain the fo
* - reply
- String
- Relaxed extJSON representation of the reply. This document MUST be truncated appropriately according to rules defined in the
`logging specification <../logging/logging.rst#configurable-max-document-length>`_, and MUST be replaced with an empty document
`logging specification <../logging/logging.rst#configurable-max-document-length>`__, and MUST be replaced with an empty document
"{ }" if the command is considered sensitive.

The unstructured form SHOULD be as follows, using the values defined in the structured format above to fill in placeholders as appropriate:
Expand Down Expand Up @@ -516,7 +516,7 @@ In addition to the common fields, command failed messages MUST contain the follo

* - failure
- Flexible
- The error. The type and format of this value is flexible; see the `logging specification <../logging/logging.rst#representing-errors-in-log-messages>`_
- The error. The type and format of this value is flexible; see the `logging specification <../logging/logging.rst#representing-errors-in-log-messages>`__
for details on representing errors in log messages. If the command is considered sensitive, the error MUST be redacted and replaced with a
language-appropriate alternative for a redacted error, e.g. an empty string, empty document, or null.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1055,7 +1055,7 @@ See the `Load Balancer Specification <../load-balancers/load-balancers.rst#event
Connection Pool Logging
~~~~~~~~~~~~~~~~~~~~~~~
Please refer to the `logging specification <../logging/logging.rst>`_ for details on logging implementations in general, including log levels, log
Please refer to the `logging specification <../logging/logging.rst>`__ for details on logging implementations in general, including log levels, log
components, handling of null values in log messages, and structured versus unstructured logging.

Drivers MUST support logging of connection pool information via the following types of log messages. These messages MUST be logged at ``Debug`` level
Expand Down Expand Up @@ -1291,7 +1291,7 @@ In addition to the common fields defined above, this message MUST contain the fo
* - error
- Flexible
- If ``reason`` is ``Error``, the associated error. The type and format of this value is flexible; see the
`logging specification <../logging/logging.rst#representing-errors-in-log-messages>`_ for details on representing errors in log messages.
`logging specification <../logging/logging.rst#representing-errors-in-log-messages>`__ for details on representing errors in log messages.

The unstructured form SHOULD be as follows, using the values defined in the structured format above to fill in placeholders as appropriate:

Expand Down Expand Up @@ -1345,7 +1345,7 @@ In addition to the common fields defined above, this message MUST contain the fo
* - error
- Flexible
- If ``reason`` is ``ConnectionError``, the associated error. The type and format of this value is flexible; see the
`logging specification <../logging/logging.rst#representing-errors-in-log-messages>`_ for details on representing errors in log messages.
`logging specification <../logging/logging.rst#representing-errors-in-log-messages>`__ for details on representing errors in log messages.

* - durationMS
- Int64
Expand Down
3 changes: 2 additions & 1 deletion source/read-write-concern/read-write-concern.rst
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,8 @@ omitted when sending ``findAndModify`` with MaxWireVersion < 4.
If the findAndModify helper accepts writeConcern as a parameter, the driver
MUST raise an error with MaxWireVersion < 4.

.. note ::
.. note::

Driver documentation SHOULD include a warning in their server 3.2
compatible releases that an elevated ``WriteConcern`` may cause
performance degradation when using ``findAndModify``. This is because
Expand Down
2 changes: 1 addition & 1 deletion source/retryable-reads/retryable-reads.rst
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ either a correlating ``CommandSucceededEvent`` or ``CommandFailedEvent`` and tha
every "command started" log message has either a correlating "command succeeded"
log message or "command failed" log message. If the first attempt of a retryable
read operation encounters a retryable error, drivers MUST fire a ``CommandFailedEvent`` and emit a "command failed" log message
for the retryable error and fire a separate ``CommandStartedEvent``and emit a separate "command started" log message when executing
for the retryable error and fire a separate ``CommandStartedEvent`` and emit a separate "command started" log message when executing
the subsequent retry attempt. Note that the second ``CommandStartedEvent`` and "command started" log message may have
a different ``connectionId``, since a server is reselected for a retry attempt.
Expand Down
4 changes: 2 additions & 2 deletions source/run-command/run-command.rst
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,8 @@ For example, default values MUST NOT be inherited from client, database, or coll

If the user-provided command document already includes ``readConcern`` or ``writeConcern`` fields, the values MUST be left as-is.

* See Read Concern's section on `Generic Command Method <https://github.com/mongodb/specifications/blob/master/source/read-write-concern/read-write-concern.rst#generic-command-method>`_
* See Write Concern's section on `Generic Command Method <https://github.com/mongodb/specifications/blob/master/source/read-write-concern/read-write-concern.rst#generic-command-method-1>`_
* See Read Concern's section on `Generic Command Method <https://github.com/mongodb/specifications/blob/master/source/read-write-concern/read-write-concern.rst#generic-command-method>`__
* See Write Concern's section on `Generic Command Method <https://github.com/mongodb/specifications/blob/master/source/read-write-concern/read-write-concern.rst#generic-command-method-1>`__

Retryability
""""""""""""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ Publishing and Subscribing

The driver SHOULD publish events in a manner that is standard to the driver's language publish/subscribe patterns and is not strictly mandated in this specification.

Similarly, as described in the `logging specification <../logging/logging.rst#implementation-requirements>`_ the driver SHOULD emit log messages in a manner that is standard for the language.
Similarly, as described in the `logging specification <../logging/logging.rst#implementation-requirements>`__ the driver SHOULD emit log messages in a manner that is standard for the language.

----------
Guarantees
Expand Down Expand Up @@ -405,7 +405,7 @@ the value to the default read preference, ``primary``, or treat the call as if `
------------
Log Messages
------------
Please refer to the `logging specification <../logging/logging.rst>`_ for details on logging implementations in general, including log levels, log
Please refer to the `logging specification <../logging/logging.rst>`__ for details on logging implementations in general, including log levels, log
components, and structured versus unstructured logging.

Drivers MUST support logging of SDAM information via the following types of log messages. These messages MUST be logged at ``Debug`` level and use
Expand Down Expand Up @@ -677,7 +677,7 @@ In addition to the relevant common fields, these messages MUST contain the follo

* - failure
- Flexible
- The error. The type and format of this value is flexible; see the `logging specification <../logging/logging.rst#representing-errors-in-log-messages>`_
- The error. The type and format of this value is flexible; see the `logging specification <../logging/logging.rst#representing-errors-in-log-messages>`__
for details on representing errors in log messages. If the command is considered sensitive, the error MUST be redacted and replaced with a
language-appropriate alternative for a redacted error, e.g. an empty string, empty document, or null.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ Outcome: Verify that the write succeeded or failed depending on existing
driver behavior with respect to the starting topology.

Monitors sleep at least minHeartbeatFrequencyMS between checks
-------------------------------------------------------------
--------------------------------------------------------------

This test will be used to ensure monitors sleep for an appropriate amount of
time between failed server checks so as to not flood the server with new
Expand Down

0 comments on commit fd24a0a

Please sign in to comment.