Read Concern Lab

Hello,

I have completed the read concern lab, user-report ticket, and found that it doesn’t actually check for read concern.
I was able to do the pipeline but once I realized that the read concern wasn’t being checked I noticed my read concern variable would have actually been wrong.

So far, none of the lectures have provided any instruction on how to actually declare read concerns.

Looking at the docs I also wasn’t totally confident that the way I was structuring the readConcern variable was correct. I initially just declared the string "majority" for the readConcern variable and all the tests passed.

I decided to write a test that checks for this and provided the needed information below. Since the answer doesn’t actually rely on setting the read concern I hope this is ok, I do not show the required aggregate command.

I would also really like some feedback on if this is a good approach to test something like this, if possible. The biggest benefit to this class, for me, is making sure I am handling these processes properly in my own applications. Perhaps, it would never be necessary to test for this at all?

First, I added a parameter with a default value to the mostActiveCommenters() function:

static async mostActiveCommenters(testing=false)

Then, I updated the return statement based on the new testing variable:

return await testing ? aggregateResult : aggregateResult.toArray()

Finally, here is the test that checks the read concern in user-report.test.js:

test("Should have majority read concern", async () => {
  const userReport = await CommentsDAO.mostActiveCommenters(true)
  expect(userReport.options.readConcern.level).toEqual("majority")
})

Writing a function that includes checks for testing seems like probably not clean code but I am not sure if there would be a better way to do it. I believe, using an environment variable would still require having a check in the function,though that may be considered a better practice? Alternatively you could return the entire results document but, returning the entire document, all of the time, also doesn’t seem ideal.

Feedback and suggestions welcome!

  • Chris
3 Likes

Hi @Christopher_79126,

Thanks for your note.

This is true and we will fix this soon.
The reason why we decided to go along with a generic readConcern lesson instead of language specific one was based on the need to explain the concept really well before diving into the actual language usage.
It also happens that read isolation in general is mostly beneficial in multi process/threading environment where we have several concurrent requests, or when you can tamper with the database itself (killing nodes, freezing filesystem, stop replication …) which using Atlas we can’t do. Moreover, in the context of mflix, it is a bit hard to do and guarantee a certain level of predictable behavior if we embark in a lab that would be meaningful.
That said, we will be adding an extra lesson explaining how to configure your driver to use readConcerns.

This could work, however at the end of the day, what is really important is to understand:

  • a) What’s the purpose and reason for using different read concerns
  • b) What’s the isolation level that each read concern offers
  • c) Where to find the information in order to set up the expect read concern in my application code.

We’ll take your feedback in mind and add more lessons and potentially add a few more labs.

Thanks once again for your feedback.

N.