Skip to content

PYTHON-4946 - Add GridFSBucket.rename_by_name #2219

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 7 commits into from
Mar 31, 2025
Merged
Show file tree
Hide file tree
Changes from 4 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
2 changes: 2 additions & 0 deletions doc/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ PyMongo 4.12 brings a number of changes including:
- Support for configuring DEK cache lifetime via the ``key_expiration_ms`` argument to
:class:`~pymongo.encryption_options.AutoEncryptionOpts`.
- Support for $lookup in CSFLE and QE supported on MongoDB 8.1+.
- Added :meth:`gridfs.asynchronous.grid_file.AsyncGridFSBucket.rename_by_name` and :meth:`gridfs.grid_file.GridFSBucket.rename_by_name`
for more performant renaming of file revisions.
Copy link
Member

Choose a reason for hiding this comment

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

of file revisions. -> of a file with multiple revisions.

- AsyncMongoClient no longer performs DNS resolution for "mongodb+srv://" connection strings on creation.
To avoid blocking the asyncio loop, the resolution is now deferred until the client is first connected.
- Added index hinting support to the
Expand Down
29 changes: 29 additions & 0 deletions gridfs/asynchronous/grid_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -1021,6 +1021,35 @@ async def rename(
"matched file_id %i" % (new_filename, file_id)
)

async def rename_by_name(
Copy link
Member

@ShaneHarvey ShaneHarvey Mar 24, 2025

Choose a reason for hiding this comment

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

Instead of adding a new method for this, should we consider overloading the existing rename method? For example:

>>> fs.rename(file_id, "new") # rename by id
>>> fs.rename(filename="old", new_filename="new") # rename by name

The pro is we have less apis to maintain. I guess a con is that the user could accidentally mix up the file_id and filename params.

Note I'm not advocating for either, I just think we should consider it and maybe ask the spec author about it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like this suggestion--both methods have nearly identical bodies, so combining them makes sense. For #2218, the two methods have slightly different bodies, but still similar enough that we could do the same thing there. Thoughts?

Copy link
Member

@ShaneHarvey ShaneHarvey Mar 25, 2025

Choose a reason for hiding this comment

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

What would the method signatures looks like (including type hints)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After trying this out a bit, I think it's a little too confusing/unclear to combine them.

Copy link
Member

Choose a reason for hiding this comment

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

Could you share the signatures for posterity?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

async def rename(file_id: Optional[Any], new_filename: str, filename: Optional[str] = None,...)

self, filename: str, new_filename: str, session: Optional[AsyncClientSession] = None
) -> None:
"""Renames the stored file with the specified filename.

For example::

my_db = AsyncMongoClient().test
fs = AsyncGridFSBucket(my_db)
await fs.upload_from_stream("test_file", "data I want to store!")
await fs.rename_by_name("test_file", "new_test_name")

Raises :exc:`~gridfs.errors.NoFile` if no file with the given filename exists.

:param filename: The filename of the file to be renamed.
:param new_filename: The new name of the file.
:param session: a :class:`~pymongo.client_session.AsyncClientSession`

.. versionadded:: 4.12
"""
_disallow_transactions(session)
result = await self._files.update_many(
{"filename": filename}, {"$set": {"filename": new_filename}}, session=session
)
if not result.matched_count:
raise NoFile(
f"no files could be renamed {new_filename!r} because none matched filename {filename!r}"
)


class AsyncGridIn:
"""Class to write data to GridFS."""
Expand Down
29 changes: 29 additions & 0 deletions gridfs/synchronous/grid_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -1015,6 +1015,35 @@ def rename(
"matched file_id %i" % (new_filename, file_id)
)

def rename_by_name(
Copy link
Member

Choose a reason for hiding this comment

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

Let's add some hand written tests for this too like test_rename in test_gridfs_bucket. That way we get some signal on typing and the UX.

self, filename: str, new_filename: str, session: Optional[ClientSession] = None
) -> None:
"""Renames the stored file with the specified filename.

For example::

my_db = MongoClient().test
fs = GridFSBucket(my_db)
fs.upload_from_stream("test_file", "data I want to store!")
fs.rename_by_name("test_file", "new_test_name")

Raises :exc:`~gridfs.errors.NoFile` if no file with the given filename exists.

:param filename: The filename of the file to be renamed.
:param new_filename: The new name of the file.
:param session: a :class:`~pymongo.client_session.ClientSession`

.. versionadded:: 4.12
"""
_disallow_transactions(session)
result = self._files.update_many(
{"filename": filename}, {"$set": {"filename": new_filename}}, session=session
)
if not result.matched_count:
raise NoFile(
f"no files could be renamed {new_filename!r} because none matched filename {filename!r}"
)


class GridIn:
"""Class to write data to GridFS."""
Expand Down
13 changes: 13 additions & 0 deletions test/asynchronous/test_gridfs_bucket.py
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,19 @@ async def test_rename(self):
b"testing", await (await self.fs.open_download_stream_by_name("second_name")).read()
)

async def test_rename_by_name(self):
_id = await self.fs.upload_from_stream("first_name", b"testing")
self.assertEqual(
b"testing", await (await self.fs.open_download_stream_by_name("first_name")).read()
)

await self.fs.rename_by_name("first_name", "second_name")
with self.assertRaises(NoFile):
await self.fs.open_download_stream_by_name("first_name")
self.assertEqual(
b"testing", await (await self.fs.open_download_stream_by_name("second_name")).read()
)

@patch("gridfs.asynchronous.grid_file._UPLOAD_BUFFER_SIZE", 5)
async def test_abort(self):
gin = self.fs.open_upload_stream("test_filename", chunk_size_bytes=5)
Expand Down
4 changes: 2 additions & 2 deletions test/asynchronous/unified_format.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
from bson import SON, json_util
from bson.codec_options import DEFAULT_CODEC_OPTIONS
from bson.objectid import ObjectId
from gridfs import AsyncGridFSBucket, GridOut
from gridfs import AsyncGridFSBucket, GridOut, NoFile
from pymongo import ASCENDING, AsyncMongoClient, CursorType, _csot
from pymongo.asynchronous.change_stream import AsyncChangeStream
from pymongo.asynchronous.client_session import AsyncClientSession, TransactionOptions, _TxnState
Expand Down Expand Up @@ -632,7 +632,7 @@ def process_error(self, exception, spec):
# Connection errors are considered client errors.
if isinstance(error, ConnectionFailure):
self.assertNotIsInstance(error, NotPrimaryError)
elif isinstance(error, (InvalidOperation, ConfigurationError, EncryptionError)):
elif isinstance(error, (InvalidOperation, ConfigurationError, EncryptionError, NoFile)):
pass
else:
self.assertNotIsInstance(error, PyMongoError)
Expand Down
Loading
Loading