Skip to content

Conversation

rustagir
Copy link
Contributor

@rustagir rustagir commented Nov 12, 2024

Pull Request Info

PR Reviewing Guidelines

JIRA - https://jira.mongodb.org/browse/DOCSP-42774
Staging - https://deploy-preview-63--docs-mongoid.netlify.app/interact-data/transaction/

Self-Review Checklist

  • Is this free of any warnings or errors in the RST?
  • Did you run a spell-check?
  • Did you run a grammar-check?
  • Are all the links working?
  • Are the facets and meta keywords accurate?

Copy link

netlify bot commented Nov 12, 2024

Deploy Preview for docs-mongoid ready!

Name Link
🔨 Latest commit e0aae73
🔍 Latest deploy log https://app.netlify.com/sites/docs-mongoid/deploys/673e49cb38beae0008688406
😎 Deploy Preview https://deploy-preview-63--docs-mongoid.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.

Copy link
Collaborator

@mayaraman19 mayaraman19 left a comment

Choose a reason for hiding this comment

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

Take a lot of these suggestions with a grain of salt because they definitely stem from my lack of Ruby knowledge.

Also, will the existing Sessions and Transactions pages be removed/redirected?

Comment on lines 39 to 45
In {+odm+}, you can perform transactions by using the following APIs:

- :ref:`mongoid-txn-high-level`: {+odm+} manages the life cycle of the
transaction. You can use this API in {+odm+} v9.0 and later.

- :ref:`mongoid-txn-low-level`: You must manage the life cycle of the
transaction. You can use this API in {+odm+} v6.4 and later.
Copy link
Collaborator

Choose a reason for hiding this comment

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

S: Feel free to ignore, but something here makes me feel like the APIs are actually named "higher level API" and "lower level API".

Suggested change
In {+odm+}, you can perform transactions by using the following APIs:
- :ref:`mongoid-txn-high-level`: {+odm+} manages the life cycle of the
transaction. You can use this API in {+odm+} v9.0 and later.
- :ref:`mongoid-txn-low-level`: You must manage the life cycle of the
transaction. You can use this API in {+odm+} v6.4 and later.
In {+odm+}, you can perform transactions by using one or both of the following APIs:
- :ref:`Higher Level <mongoid-txn-high-level>`: {+odm+} manages the life cycle of the
transaction. You can use this API in {+odm+} v9.0 and later.
- :ref:`Lower Level <mongoid-txn-low-level>`: You must manage the life cycle of the
transaction. You can use this API in {+odm+} v6.4 and later.

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'll get a clarification on this

@rustagir rustagir requested a review from mayaraman19 November 13, 2024 17:30
Copy link
Collaborator

@mayaraman19 mayaraman19 left a comment

Choose a reason for hiding this comment

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

LGTM! left a couple small suggestions.

Also, still curious on how existing Sessions/Transactions pages will fit into this!

:dedent:

To learn more about the available session options, see the
:ruby-api:`Session class constructor details
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: API links are broken until the next snooty parser release see mongodb/snooty-parser#633

# Starts a session from an instance of Book
book.with_session do |session|
# Starts the transaction in the session
session.start_transaction
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not sure whether this and following two examples make sense. We should call session.start_transaction and session.commit_transaction or session.abort_transaction within the same with_session block. Maybe we should combine there examples into one.

Copy link
Contributor Author

@rustagir rustagir Nov 18, 2024

Choose a reason for hiding this comment

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


.. _mongoid-txn-convenient:

Convenient Transaction API
Copy link
Collaborator

Choose a reason for hiding this comment

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

The term "Convenient Transaction API" refers to the drivers API. It can be used in Mongoid like this (but this it probably not the best idea):

person.with_session do |session|
  session.with_transaction do
    person.save!
  end
end

Maybe we should not using this term to avoid confusion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reverted the terminology to use high-level and low-level

Copy link
Collaborator

@comandeo-mongo comandeo-mongo left a comment

Choose a reason for hiding this comment

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

LGTM, just one small change seems to be necessary.

Co-authored-by: Dmitry Rybakov <[email protected]>
@rustagir rustagir merged commit 95545a2 into mongodb:standardized Nov 20, 2024
4 of 5 checks passed
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