Skip to content

Commit

Permalink
PYTHON-5002 Add guard to synchro hook to accidental overwrite (#2026)
Browse files Browse the repository at this point in the history
  • Loading branch information
blink1073 authored Dec 4, 2024
1 parent dc34833 commit 5204e87
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 2 deletions.
1 change: 1 addition & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ repos:
entry: bash ./tools/synchro.sh
language: python
require_serial: true
fail_fast: true
additional_dependencies:
- ruff==0.1.3
- unasync
Expand Down
12 changes: 12 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,18 @@ you are attempting to validate new spec tests in PyMongo.

Follow the [Python Driver Release Process Wiki](https://wiki.corp.mongodb.com/display/DRIVERS/Python+Driver+Release+Process).

## Asyncio considerations

PyMongo adds asyncio capability by modifying the source files in `*/asynchronous` to `*/synchronous` using
[unasync](https://github.com/python-trio/unasync/) and some custom transforms.

Where possible, edit the code in `*/asynchronous/*.py` and not the synchronous files.
You can run `pre-commit run --all-files synchro` before running tests if you are testing synchronous code.

To prevent the `synchro` hook from accidentally overwriting code, it first checks to see whether a sync version
of a file is changing and not its async counterpart, and will fail.
In the unlikely scenario that you want to override this behavior, first export `OVERRIDE_SYNCHRO_CHECK=1`.

## Converting a test to async
The `tools/convert_test_to_async.py` script takes in an existing synchronous test file and outputs a
partially-converted asynchronous version of the same name to the `test/asynchronous` directory.
Expand Down
15 changes: 15 additions & 0 deletions tools/synchro.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@

from __future__ import annotations

import os
import re
import sys
from os import listdir
from pathlib import Path

Expand Down Expand Up @@ -356,6 +358,19 @@ def unasync_directory(files: list[str], src: str, dest: str, replacements: dict[


def main() -> None:
modified_files = [f"./{f}" for f in sys.argv[1:]]
errored = False
for fname in async_files + gridfs_files:
# If the async file was modified, we don't need to check if the sync file was also modified.
if str(fname) in modified_files:
continue
sync_name = str(fname).replace("asynchronous", "synchronous")
if sync_name in modified_files and "OVERRIDE_SYNCHRO_CHECK" not in os.environ:
print(f"Refusing to overwrite {sync_name}")
errored = True
if errored:
raise ValueError("Aborting synchro due to errors")

unasync_directory(async_files, _pymongo_base, _pymongo_dest_base, replacements)
unasync_directory(gridfs_files, _gridfs_base, _gridfs_dest_base, replacements)
unasync_directory(test_files, _test_base, _test_dest_base, replacements)
Expand Down
6 changes: 4 additions & 2 deletions tools/synchro.sh
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#!/bin/bash -eu
#!/bin/bash

python ./tools/synchro.py
set -eu

python ./tools/synchro.py "$@"
python -m ruff check pymongo/synchronous/ gridfs/synchronous/ test/ --fix --silent
python -m ruff format pymongo/synchronous/ gridfs/synchronous/ test/ --silent

0 comments on commit 5204e87

Please sign in to comment.