Skip to content
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

Add support for AWS DocumentDB #304

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add support for AWS DocumentDB #304

wants to merge 2 commits into from

Conversation

jethro-solve
Copy link

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.

@sourcelevel-bot
Copy link

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.

@foggy1
Copy link

foggy1 commented Sep 10, 2019

@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
Copy link

foggy1 commented Sep 10, 2019

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.

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