PYTHON-5981 Reject aggregate/pipeline as aggregation options - #2998
Merged
Conversation
The reserved-option tests only asserted that ConfigurationError is raised, so one of them passed for the wrong reason: list_search_indexes(aggregate=...) raises OperationFailure on a non-Atlas server before the option is applied. Assert the behavior instead. Command monitoring shows the supplied namespace never reaches the wire, which holds on any topology, and separate tests show a supplied pipeline can neither read ($unionWith) nor overwrite ($out) another collection.
Both tests are blocked by the same ConfigurationError as test_aggregate_reserved_options, so they cannot fail independently.
Co-authored-by: Noah Stapp <noah.stapp@mongodb.com>
aclark4life
force-pushed
the
PYTHON-5981
branch
from
August 18, 2026 20:38
d8c8959 to
f7f1180
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens aggregation command construction by rejecting reserved aggregate-command fields (aggregate, pipeline) when supplied via keyword options, preventing user kwargs from silently overriding the generated command’s target namespace or pipeline.
Changes:
- Added reserved-option validation in
_AggregationCommand.__init__for both async and generated sync implementations. - Added regression tests covering
aggregate=across aggregation helpers andpipeline=forlist_search_indexes(async + generated sync tests). - Documented the behavior change in the 4.18.0 changelog.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/test_collection.py | Adds (generated sync) regression test for rejecting reserved aggregation options. |
| test/asynchronous/test_collection.py | Adds source async regression test for rejecting reserved aggregation options. |
| pymongo/synchronous/aggregation.py | Rejects aggregate/pipeline in aggregation options (generated sync). |
| pymongo/asynchronous/aggregation.py | Rejects aggregate/pipeline in aggregation options (source async). |
| doc/changelog.rst | Adds changelog entry for the new ConfigurationError behavior (note: title formatting needs correction per review comment). |
Suppressed comments (1)
doc/changelog.rst:3
- The top-level RST title underline is too short ("=="), and the blank line after the title was removed. This is likely to trigger a Sphinx/docutils error like "Title underline too short" and break the docs build. Restore the proper underline length and the blank line after the title.
Changelog
=========
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
blink1073
reviewed
Aug 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PYTHON-5981
Changes in this PR
_AggregationCommand.__init__now raisesConfigurationErrorifaggregateorpipelineis passed as a keyword option.Both names are fields of the
aggregatecommand itself, so previously passing them as options silently replaced the generated command's target namespace or its pipeline rather than adding an option. They now joinexplainas rejected option names, with the messageThe <name> option cannot be specified as a keyword argument.This affects the helpers that build an aggregate command:
AsyncCollection.aggregate/Collection.aggregateAsyncCollection.aggregate_raw_batches/Collection.aggregate_raw_batchesAsyncDatabase.aggregate/Database.aggregateAsyncCollection.list_search_indexes/Collection.list_search_indexesTest Plan
Added
test_aggregate_reserved_optionstotest/asynchronous/test_collection.pycoveringaggregate=on all four helpers pluspipeline=onlist_search_indexes, assertingConfigurationErrorin each case.Checklist
Checklist for Author
Checklist for Reviewer