DOCSP-15571 productize new async writer #199

ghost · 2022-03-30T19:58:34Z

STAGING

https://docs-mongodbcom-staging.corp.mongodb.com/mongodb-shell/docsworker-xlarge/DOCSP-15571-productize-new-async-rewriter/reference/compatibility/#limitations-on-database-calls
https://docs-mongodbcom-staging.corp.mongodb.com/mongodb-shell/docsworker-xlarge/DOCSP-15571-productize-new-async-rewriter/write-scripts/

JIRA

https://jira.mongodb.org/browse/DOCSP-15571
[MONGOSH] Productize new async rewriter

jmd-mongo

Thanks for the changes, @davemungo! I've left some comments and questions for you.

Thanks!
Joe

jmd-mongo · 2022-03-31T14:37:20Z

source/includes/fact-call-limitations.rst

+Constructors
+~~~~~~~~~~~~
+
+The following constructors will not work: 


[suggestion]

Consider using the present tense.

jmd-mongo · 2022-03-31T14:38:17Z

source/includes/fact-call-limitations.rst

+     return db[ collectionOne ].estimatedDocumentCount() - db[ collectionOne ].estimatedDocumentCount() )
+   } );
+
+Use ``.map`` instead. 


[suggestion]

I suggest adding the () for consistency with usage on line 14.

jmd-mongo · 2022-03-31T14:41:28Z

source/includes/fact-call-limitations.rst

@@ -0,0 +1,104 @@
+.. Top level title in calling page, won't render in right TOC
+
+The results of database calls cannot be passed inside the following


[question]

Are we expecting users to already be familiar with the terminology "database calls"?

I see we're using .find() ops as an examples throughout -- are there other common types of calls that ought to spring to mind?

Yes, I think this page really only applies for users with a fair amount of experience. Changed to 'queries' though as a clarification.

I'm not sure about other operations, I've added findMany(), but that's pretty much the same thing. find() is perhaps the most obvious because it will return data that you want to do something with. I can raise the question on tech review.

jmd-mongo · 2022-03-31T14:42:39Z

source/includes/fact-call-limitations.rst

+<https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for-await...of>`__,
+or ``.map()``.
+
+Constructors


[praise]

I like the "do not use this approach" vs "instead, use this approach" structure here. Clean!

jmd-mongo · 2022-03-31T14:46:07Z

source/includes/fact-call-limitations.rst

+Generator Functions
+~~~~~~~~~~~~~~~~~~~
+
+The following generator functions will not work: 


Similar comment as line 19.

jmd-mongo · 2022-03-31T14:46:17Z

source/includes/fact-call-limitations.rst

+Array Sort
+~~~~~~~~~~
+
+The following array sort will not work: 


Similar comment as lines 19, 54.

ghost

@jmd-mongo thanks for your feedback, this is ready for another pass.

ghost · 2022-03-31T15:34:09Z

source/includes/fact-call-limitations.rst

@@ -0,0 +1,104 @@
+.. Top level title in calling page, won't render in right TOC
+
+The results of database calls cannot be passed inside the following


Yes, I think this page really only applies for users with a fair amount of experience. Changed to 'queries' though as a clarification.

I'm not sure about other operations, I've added findMany(), but that's pretty much the same thing. find() is perhaps the most obvious because it will return data that you want to do something with. I can raise the question on tech review.

ghost · 2022-03-31T15:34:38Z

source/includes/fact-call-limitations.rst

+<https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for-await...of>`__,
+or ``.map()``.
+
+Constructors


ghost · 2022-03-31T15:34:56Z

source/includes/fact-call-limitations.rst

+Constructors
+~~~~~~~~~~~~
+
+The following constructors will not work: 


ghost · 2022-03-31T15:35:25Z

source/includes/fact-call-limitations.rst

+Generator Functions
+~~~~~~~~~~~~~~~~~~~
+
+The following generator functions will not work: 


ghost · 2022-03-31T15:35:32Z

source/includes/fact-call-limitations.rst

+Array Sort
+~~~~~~~~~~
+
+The following array sort will not work: 


ghost · 2022-03-31T15:35:49Z

source/includes/fact-call-limitations.rst

+     return db[ collectionOne ].estimatedDocumentCount() - db[ collectionOne ].estimatedDocumentCount() )
+   } );
+
+Use ``.map`` instead. 


jmd-mongo

Thanks for the changes, @davemungo! This one LGTM.

Thanks,
Joe

ghost · 2022-03-31T16:40:49Z

@addaleax Would you have a look at these updates?

Two technical questions -
Is it ok to say, "database queries" instead of "database calls"?

The examples use a find() query. Does it make sense to show an example with some other sort of query?

Thanks!

addaleax

Is it ok to say, "database queries" instead of "database calls"?

I think you can do that – but keep in mind that this applies to anything that interacts with the database in some way (for example, commands rather than queries).

The examples use a find() query. Does it make sense to show an example with some other sort of query?

I don’t think you’d need to provide explicit examples here – the limitations and workarounds would apply identically to any other database call (or database query 🙂)

source/includes/fact-call-limitations.rst

ghost

@addaleax Thanks for your feedback. I added a short note about the aysnch alternative.

source/includes/fact-call-limitations.rst

jason-price-mongodb

@davemungo please address mistake and resubmit PR. Cheers.

jason-price-mongodb · 2022-04-01T16:12:15Z

source/includes/fact-call-limitations.rst

+Array Sort
+~~~~~~~~~~
+
+The following array sort do not work: 


"does not work"

ghost

Thanks @jason-price-mongodb

I've updated "do not" to "does not"

DOCSP-15571 productize new async writer

c838d46

jmd-mongo reviewed Mar 31, 2022

View reviewed changes

Review feedback

a14fb7c

ghost commented Mar 31, 2022

View reviewed changes

jmd-mongo approved these changes Mar 31, 2022

View reviewed changes

ghost requested a review from addaleax March 31, 2022 16:37

addaleax approved these changes Apr 1, 2022

View reviewed changes

source/includes/fact-call-limitations.rst Show resolved Hide resolved

mungitoperrito added 2 commits April 1, 2022 09:32

Review comments

3e81545

Staging fixes

caf9fc2

ghost commented Apr 1, 2022

View reviewed changes

source/includes/fact-call-limitations.rst Show resolved Hide resolved

addaleax approved these changes Apr 1, 2022

View reviewed changes

jason-price-mongodb requested changes Apr 1, 2022

View reviewed changes

Merge fix

b5a388d

ghost commented Apr 1, 2022

View reviewed changes

jason-price-mongodb merged commit 8f11c41 into mongodb:master Apr 1, 2022

		@@ -0,0 +1,104 @@
		.. Top level title in calling page, won't render in right TOC

		The results of database calls cannot be passed inside the following

DOCSP-15571 productize new async writer #199

DOCSP-15571 productize new async writer #199

Uh oh!

Conversation

ghost commented Mar 30, 2022

STAGING

JIRA

Uh oh!

jmd-mongo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmd-mongo left a comment

Choose a reason for hiding this comment

Uh oh!

ghost commented Mar 31, 2022

Uh oh!

addaleax left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ghost left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jason-price-mongodb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!