Skip to content

DOCSP-42298: Stable API #76

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 6 commits into from
Aug 22, 2024
Merged

Conversation

mcmorisi
Copy link
Collaborator

Pull Request Info

PR Reviewing Guidelines

JIRA - https://jira.mongodb.org/browse/DOCSP-42298
Staging - https://preview-mongodbmcmorisi.gatsbyjs.io/java-rs/DOCSP-42298-stable-api/connect-to-mongo/stable-api/

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 Aug 20, 2024

Deploy Preview for docs-java-rs ready!

Name Link
🔨 Latest commit 3dfaae7
🔍 Latest deploy log https://app.netlify.com/sites/docs-java-rs/deploys/66c77adbb2d28a000817c258
😎 Deploy Preview https://deploy-preview-76--docs-java-rs.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

@jordan-smith721 jordan-smith721 left a comment

Choose a reason for hiding this comment

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

Nice work! Just a couple things but otherwise LGTM

- Description

* - ``strict()``
- | **Optional**. When ``True``, if you call a command that isn't part of
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- | **Optional**. When ``True``, if you call a command that isn't part of
- | **Optional**. When ``true``, if you call a command that isn't part of

| Default: **False**

* - ``deprecationErrors()``
- | **Optional**. When ``True``, if you call a command that is deprecated in the
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- | **Optional**. When ``True``, if you call a command that is deprecated in the
- | **Optional**. When ``true``, if you call a command that is deprecated in the

- | **Optional**. When ``True``, if you call a command that isn't part of
the declared API version, the driver raises an exception.
|
| Default: **False**
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
| Default: **False**
| Default: **false**

- | **Optional**. When ``True``, if you call a command that is deprecated in the
declared API version, the driver raises an exception.
|
| Default: **False**
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
| Default: **False**
| Default: **false**

Comment on lines 94 to 95
The following code example shows how you can set configurations on an instance of ``ServerApi``
by chaining methods on the ``ServerApi.Builder``:
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
The following code example shows how you can set configurations on an instance of ``ServerApi``
by chaining methods on the ``ServerApi.Builder``:
The following code example shows how you can configure an instance of ``ServerApi``
by chaining methods on the ``ServerApi.Builder``:

Comment on lines 114 to 115
Provided apiStrict:true, but the command count is not in API Version
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be a general operation instead of directly applying to count?

Suggested change
Provided apiStrict:true, but the command count is not in API Version
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Provided apiStrict:true, but the command <operation> is not in API Version
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

@mcmorisi mcmorisi requested review from a team and jyemin and removed request for a team August 20, 2024 14:53

The {+stable-api+} feature forces the server to run operations with behaviors compatible
with the API version you specify. When you update either your driver or server,
the API version changes, which can change the way these operations behave.
Copy link

Choose a reason for hiding this comment

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

When you update either your driver or server, the API version changes, which can change the way these operations behave.

This is not accurate. Was this text copied from somewhere else?

https://www.mongodb.com/docs/manual/reference/stable-api/ looks accurate to me, so we should either link to it or crib from it.

Copy link
Collaborator Author

@mcmorisi mcmorisi Aug 22, 2024

Choose a reason for hiding this comment

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

This description was taken from the same page from a different driver, yes. Can take this back to the team to investigate whether there's a larger-scope misunderstanding here.

In the meantime, can remove the sentence you called out, as I believe the remaining copy adequately explains the feature (plus we have the link to the server manual).


Once you create a ``MongoClient`` instance with
a specified API version, all commands you run with the client use the specified
version. If you must run commands using more than one version of the
Copy link

Choose a reason for hiding this comment

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

Currently there is only one version of the Stable API: V1.

Consider changing this to refer to one of the other properties of the ServerApi class, e.g. strict.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do we expect another version to release? It could benefit us to cover our bases in advance of future versions.

I can also tweak the last highlighted sentence to the following more general version:

If you must run Stable API commands using alternative configurations, create a new MongoClient.

Copy link

Choose a reason for hiding this comment

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

I have heard nothing about a new version coming out.

@mcmorisi mcmorisi requested a review from jyemin August 22, 2024 14:20
Copy link

@jyemin jyemin left a comment

Choose a reason for hiding this comment

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

LGTM assuming you accept the suggestion.

{+stable-api+}, create a new ``MongoClient``.
Once you create a ``MongoClient`` instance with the {+stable-api+}, all commands you
run with the client use the specified {+stable-api+} configuration. If you must run
{+stable-api+} commands using alternative configurations, create a new ``MongoClient``.
Copy link

Choose a reason for hiding this comment

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

Suggested change
{+stable-api+} commands using alternative configurations, create a new ``MongoClient``.
commands using alternative configurations, create a new ``MongoClient``.

Because you might want to run commands that are not part of the Stable API.

@mcmorisi mcmorisi merged commit 981dfbb into mongodb:main Aug 22, 2024
5 checks passed
@mcmorisi mcmorisi deleted the DOCSP-42298-stable-api branch August 22, 2024 17:55
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