mongodb icon indicating copy to clipboard operation
mongodb copied to clipboard

Add support for AWS DocumentDB

Open jethro-solve opened this issue 5 years ago • 3 comments

As per thread in #273, there are some changes required to add support for DocumentDB. These were written by @AlimovSV originally, I've just re-factored them to get a mergeable PR.

Note that I'm not properly across what is going on in this code, so it warrants careful attention and testing.

jethro-solve avatar Jul 17 '19 21:07 jethro-solve

Hello, @jethro-solve! This is your first Pull Request that will be reviewed by SourceLevel, an automatic Code Review service. It will leave comments on this diff with potential issues and style violations found in the code as you push new commits. You can also see all the issues found on this Pull Request on its review page. Please check our documentation for more information.

sourcelevel-bot[bot] avatar Jul 17 '19 21:07 sourcelevel-bot[bot]

@jethro-solve I may make a PR if I manage to track down what's happening (I literally just discovered this and am happy it had an explanation!) but this PR will break the work cursor.ex is doing because in line 77 of that file when it runs Mongo.get_more the new returned value appears to match what should be returned by the first batch but not what should be returned from the "next" batches.

I'm going to get more specific shortly but just wanted to shout about it because I'm excited.

foggy1 avatar Sep 10 '19 22:09 foggy1

To be clear, Mongo.Cursor.next_fun/2 has a pattern match that goes as such:

case Mongo.get_more(conn, coll, cursor, opts) do
            {:ok, %{"cursor" => %{"id" => cursor, "nextBatch" => []}}} ->
              {:halt, state(state, cursor: cursor)}

            {:ok, %{"cursor" => %{"id" => cursor, "nextBatch" => docs}}} ->
              {docs, state(state, cursor: cursor, limit: new_limit(limit, batch_size))}

            {:error, error} ->
              raise error
          end

But the proposed changes make it so that Mongo.get_more returns something looking like:

{:ok, %{from: 0, num: Enum.count(docs), cursor_id: id, docs: docs}}

So you get a no case clause matching error on anything that needs more than reduces more than the first response from the cursor.

foggy1 avatar Sep 10 '19 22:09 foggy1