Skip to content

PYTHON-5002 Add guard to synchro hook to accidental overwrite #2026

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Dec 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we change this to check that the async name is not in modified files? This code is a little confusing to read.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a comment

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
Loading