Skip to content

Conversation

juliusgeo
Copy link
Contributor

No description provided.

@juliusgeo
Copy link
Contributor Author

I verified that it renders correctly by doing building the docs locally with sphinx.


When one attempts to connect to a <=3.4 version server, PyMongo will throw the following error:

.. code-block::
Copy link
Member

Choose a reason for hiding this comment

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

You can remove these .. code-block:: lines in favor of ::. For example:

... following error::

  >>> client.admin.command('ping')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


.. code-block::
>>> client.admin.command('ping')
Copy link
Member

Choose a reason for hiding this comment

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

For all the code examples, indent the lines only 2 spaces like we do for most other code examples.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Member

Choose a reason for hiding this comment

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

It still looks like these examples are intended one tab instead of 2 spaces.


PyMongo will no longer support :meth:`pymongo.cursor.count` in the near future.
Instead, use :meth:`pymongo.collection.count_documents`.
Note that this is NOT the same as ``Cursor.count_documents`` (which does not exist),
Copy link
Member

Choose a reason for hiding this comment

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

We should show the AttributeError: 'Cursor' object has no attribute 'count_documents' error as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

This is due to the fact that PyMongo discovers replica set members using the response from the isMaster command which
then contains the address and ports of the other members. However, these addresses and ports will not be accessible through the SSH tunnel. Thus, this behavior is unsupported.
You can, however, successfully connect to a single MongoDB node using SSH tunneling.
Copy link
Member

Choose a reason for hiding this comment

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

"connect directly to a single MongoDB node using with the directConnection=True option with SSH tunneling."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Note that this is NOT the same as ``Cursor.count_documents`` (which does not exist),
this is a method of the Collection class, so you must call it on a collection object::

>>> Cursor(MongoClient().db.coll).count()
Copy link
Member

Choose a reason for hiding this comment

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

Should this be .count_documents() instead of .count()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I understand correctly, the original Cursor method was called count, and the collection method is called count_documents. So in this case, because I'm illustrating the Cursor method it is just count.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see, so perhaps put the note before and then after say that if you try to use the count property you will see this error:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@juliusgeo juliusgeo dismissed stale reviews from blink1073 and ShaneHarvey April 5, 2022 07:50

Updated code

Copy link
Member

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

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

LGTM!

@@ -0,0 +1,98 @@
Frequently Encountered Issues
=============================
Also see the `TLS Examples <https://pymongo.readthedocs.io/en/stable/examples/tls.html>`_.
Copy link
Member

Choose a reason for hiding this comment

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

This should be an RST link to the "Troubleshooting TLS Errors" section. You can add a cross doc link like this:


.. _TLSErrors:

Troubleshooting TLS Errors
..........................

------------------------------------

MongoClient has historically accepted CodecOptions's members as keyword arguments,
however this does not work for type_registry in PyMongo <3.8::
Copy link
Member

Choose a reason for hiding this comment

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

This sentence should be updated now that were only talking about incorrect kwargs in general.

@juliusgeo juliusgeo requested a review from ShaneHarvey April 5, 2022 19:30
@juliusgeo juliusgeo force-pushed the PYTHON-3080 branch 3 times, most recently from 26653eb to 29378ba Compare April 6, 2022 07:27
@juliusgeo juliusgeo requested a review from blink1073 April 6, 2022 07:27

.. code-block::
>>> client.admin.command('ping')
Copy link
Member

Choose a reason for hiding this comment

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

It still looks like these examples are intended one tab instead of 2 spaces.

python3
migrate-to-pymongo4
developer/index
common-issues
Copy link
Member

Choose a reason for hiding this comment

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

This tree is hidden. We need to add a link to the new page in the "Overview" section otherwise it doesn't show up: https://pymongo--918.org.readthedocs.build/en/918/#overview

:doc:`developer/index`
Developer guide for contributors to PyMongo.

:doc:`common-issues`
Copy link
Member

Choose a reason for hiding this comment

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

The new rst file still has to be included in the toctree at the bottom of the file.

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 think I have corrected it.

@juliusgeo juliusgeo force-pushed the PYTHON-3080 branch 2 times, most recently from 59b4732 to f2e96a4 Compare April 7, 2022 11:26
@juliusgeo
Copy link
Contributor Author

My git configuration for this branch is very messed up--I think I'm just going to blow it all away and restart.

Copy link
Member

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

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

👍🏼

Copy link
Member

@ShaneHarvey ShaneHarvey left a comment

Choose a reason for hiding this comment

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

Could you fix the following 3 build warnings? (the first 2 are existing issues but we might as well fix it here):

/home/docs/checkouts/readthedocs.org/user_builds/pymongo/checkouts/918/gridfs/grid_file.py:docstring of gridfs.grid_file.GridOut.__iter__:4: WARNING: Block quote ends without a blank line; unexpected unindent.
/home/docs/checkouts/readthedocs.org/user_builds/pymongo/checkouts/918/bson/codec_options.py:docstring of bson.codec_options.CodecOptions:35: WARNING: unknown document: examples/uuid
/home/docs/checkouts/readthedocs.org/user_builds/pymongo/checkouts/918/doc/common-issues.rst:4: WARNING: undefined label: tlserrors (if the link has no caption the label must precede a section header)

https://readthedocs.org/projects/pymongo/builds/16588181/

@blink1073
Copy link
Member

Huh, we have fail_on_warning set in our readthedocs config, not sure why that isn't being honored.


def __iter__(self) -> "GridOut":
"""Return an iterator over all of this file's data.
r"""Return an iterator over all of this file's data.
Copy link
Member

Choose a reason for hiding this comment

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

Please remove the changes to this file. I already fixed this last September in PYTHON-2046. In the future please make sure you branch from the latest commit. For example:

git checkout master
git pull upstream master
git push
git checkout -b PYTHON-XXXX

@juliusgeo juliusgeo requested a review from ShaneHarvey April 13, 2022 18:21
@ShaneHarvey ShaneHarvey merged commit 868b3f7 into mongodb:master Apr 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants