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

support the $changeStream aggregation in 3.6+ #38

Closed
wants to merge 3 commits into from

Conversation

domodwyer
Copy link

@domodwyer domodwyer commented Sep 15, 2017

The $changeStream aggregation isn't very well documented, but looks very interesting. An overview can be found here:

https://mongodb.github.io/mongo-java-driver/3.6/driver/tutorials/change-streams/

This commit is a squash of 4 commits from the 10gen fork of mgo:
* go-mgo@e78a135
* go-mgo@31a6d57
* go-mgo@7940d20
* go-mgo@0d8351c

The $changeStream aggregation isn't very well documented, but an overview can be
found here:

	https://mongodb.github.io/mongo-java-driver/3.6/driver/tutorials/change-streams/

This commit is a squash of 4 commits from the 10gen fork of mgo:
	* e78a135
	* 31a6d57
	* 7940d20
	* 0d8351c
Copy link

@szank szank left a comment

Choose a reason for hiding this comment

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

We are missing the values that can be set in the FullDocument field.
This is:

  • DEFAULT
  • UPDATE_LOOKUP
    And possibly some documentation re ResumeAfter field.
    I assume this is a query that would return a single document, no?

@domodwyer
Copy link
Author

domodwyer commented Sep 18, 2017

Keeping this on hold for the time being - ideally we want to wait for $changeStream to become a little better documented.

@amineck
Copy link

amineck commented Dec 1, 2017

Any update on this thread ?
Thank you

@domodwyer
Copy link
Author

domodwyer commented Dec 4, 2017

Hi @surfeurX

I think it's definitely mature enough to consider implementing now the 3.6 release is imminent. Some helpful integration docs have appeared too:

Our team is rather busy at the moment so we're open to outside PR's for this I guess - it's our intention to get this feature in one way or another though, I just can't put a timeline on it yet.

Dom

@amineck
Copy link

amineck commented Dec 5, 2017

Thank you @domodwyer, I may have some time to implement it next week 👍

@mcmurdont
Copy link

We are looking at implementing this too. We will review the docs tomorrow, 2017-12-07, and come up with a testing strategy.

@IAD IAD mentioned this pull request Dec 22, 2017
@domodwyer domodwyer mentioned this pull request Jan 20, 2018
@peterdeka
Copy link

Any news on this? Are just tests needed or is there something else missing?
I can do my best to write a couple of tests in case.

@peterdeka
Copy link

peterdeka commented Feb 2, 2018

If i understand correctly, Next() is currently a blocking function and the MaxAwaitTimeMS parameter is a mock, it is not used. The code is pretty useless right now as a full blocking function is rarely useful in real case scenarios. We should make use of the MaxAwaitTimeMS, if i understand correctly this is a deeper option of the iterator cursor. Any help/idea on how to proceed down this way? It seems the driver is not ready for this type of options.

@peterdeka
Copy link

Opened PR #97 hoping we can move on.

@domodwyer
Copy link
Author

Hi @peterdeka - really sorry for the lack of a reply, I've been away at a conference.

Thanks for opening a PR - it's really appreciated! We'll take a look 👍

Dom

@domodwyer domodwyer closed this Feb 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants