Skip to content

Update limitations for 5.1.0b3 release #32

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 1 commit into from
May 8, 2025

Conversation

timgraham
Copy link
Collaborator

@timgraham timgraham commented May 8, 2025

Staging Links

  • limitations-upcoming
  • Copy link

    netlify bot commented May 8, 2025

    Deploy Preview for docs-django ready!

    Name Link
    🔨 Latest commit 39d9ffe
    🔍 Latest deploy log https://app.netlify.com/sites/docs-django/deploys/681cc4acaef51400084806ce
    😎 Deploy Preview https://deploy-preview-32--docs-django.netlify.app
    📱 Preview on mobile
    Toggle QR Code...

    QR Code

    Use your smartphone camera to open QR code link.

    To edit notification comments on pull requests, go to your Netlify site configuration.

    @timgraham
    Copy link
    Collaborator Author

    Please approve the deploy request and confirm whether or not the intersphinx links work. I'm not sure exactly where we are on that (#28). The syntax is copied from the django-mongodb-backend docs, but it seems there may still be some incompatibilities (bugs?) with the MongoDB documentation builder.

    @timgraham timgraham requested a review from norareidy May 8, 2025 02:19
    Copy link
    Collaborator

    @norareidy norareidy left a comment

    Choose a reason for hiding this comment

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

    Left some comments!

    Comment on lines 207 to 208
    Database caching uses this {+django-odm+}'s ``createcachecollection`` command
    rather Django's SQL-specific ``createcachetable``.
    Copy link
    Collaborator

    Choose a reason for hiding this comment

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

    S: some small wording suggestions:

    Suggested change
    Database caching uses this {+django-odm+}'s ``createcachecollection`` command
    rather Django's SQL-specific ``createcachetable``.
    Database caching uses {+django-odm+}'s ``createcachecollection`` command
    rather than {+framework+}'s SQL-specific ``createcachetable`` command.

    rather Django's SQL-specific ``createcachetable``.

    Secondly, you must use the ``django_mongodb_backend.cache.MongoDBCache``
    backend rather than Django's built-in database cache backend,
    Copy link
    Collaborator

    Choose a reason for hiding this comment

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

    S:

    Suggested change
    backend rather than Django's built-in database cache backend,
    backend rather than {+framework+}'s built-in database cache backend,

    Copy link
    Collaborator Author

    @timgraham timgraham May 8, 2025

    Choose a reason for hiding this comment

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

    Uff, using a 13 character macro instead of "Django" seems tedious! Why not a macro for "MongoDB"? ;-)

    Copy link
    Collaborator

    Choose a reason for hiding this comment

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

    agreed it seems a little unnecessary but we try to use the macros as much as possible for product names

    Comment on lines 174 to 175
    :class:`~django.db.models.functions.TruncDate` and
    :class:`~django.db.models.functions.TruncTime` database functions isn't
    Copy link
    Collaborator

    Choose a reason for hiding this comment

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

    I: these links aren't working; can you try using the :py:func: role instead?

    Copy link
    Collaborator

    Choose a reason for hiding this comment

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

    i.e. :py:func:~django.db.models.functions.TruncDate and :py:func:~django.db.models.functions.TruncTime

    Copy link
    Collaborator Author

    Choose a reason for hiding this comment

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

    These are classes, not functions. I guess the py prefix is required though.

    @@ -181,7 +182,7 @@ Unsupported Management Commands

    The following ``django-admin`` commands are unsupported:

    - ``createcachetable``
    - ``createcachetable`` (see :ref:`django-limitations-caching`)
    Copy link
    Collaborator Author

    Choose a reason for hiding this comment

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

    Looking at the rendered docs, there appears to be a docs platform bug where the closing parenthesis is included in the link. Could you report it?

    Copy link
    Collaborator

    @norareidy norareidy May 8, 2025

    Choose a reason for hiding this comment

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

    interesting, I will let the platform team know. You might be able to get around it if you use :ref:Caching <django-limitations-caching>

    Copy link
    Collaborator Author

    Choose a reason for hiding this comment

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

    I don't think it matters enough to try a workaround. I'll trust the bug will be fixed soon enough. Thanks!

    Copy link
    Collaborator

    @norareidy norareidy left a comment

    Choose a reason for hiding this comment

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

    LGTM!

    @timgraham timgraham merged commit 39d9ffe into mongodb:master May 8, 2025
    5 checks passed
    @timgraham timgraham deleted the 5.1.0b3 branch May 8, 2025 18:20
    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.

    2 participants