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

Updated cats and monix dependencies #69

Closed

Conversation

ilya-murzinov
Copy link

As requested in #68
Tests are failing right now, I'll fix them

): FetchMonadError[Future] = new FetchMonadError[Future] {
override def runQuery[A](j: Query[A]): Future[A] = j match {
case Sync(e) => ME.pureEval(e)
case Sync(e) => pure(e.value)
Copy link
Contributor

@raulraja raulraja Oct 19, 2016

Choose a reason for hiding this comment

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

We were before abusing pureEval to lift lazy computations potentially effectful to M[_] but this will eagerly evaluate them and place the result in M which is not what we want for Future and others.
We need something in the lines of...

@typeclass trait Capture[F[_]] {
   def capture[A](a :=> A) : F[A]
}

with instances for the supported target M[_], so computation are lazily placed inside context that can capture effects.

Copy link
Author

Choose a reason for hiding this comment

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

@raulraja thanks for the hint!
Is is possible that this change is causing tests to fail?

[info] - Every level of joined concurrent fetches is combined and batched *** FAILED ***
[info]   (1,1,6) did not equal (3,3,6) (FetchTests.scala:481)
[info] - The product of concurrent fetches of the same type implies everything fetched in a single batch *** FAILED ***
[info]   (1,1,4) did not equal (2,1,4) (FetchTests.scala:457)
[info] - Every level of sequenced concurrent of concurrent fetches is batched *** FAILED ***
[info]   (5,5,19) did not equal (3,3,19) (FetchTests.scala:508)

Copy link

Choose a reason for hiding this comment

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

Thanks for taking the time to make these changes Ilya, I've made some changes to the fetch execution reporting that should make those errors go away in the reporting branch. I'm going to try to combine your work and mine and cut a release as soon as possible.

Copy link
Author

Choose a reason for hiding this comment

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

@dialelo ooooops, I didn't see that it' already implemented in your branch :(

@ilya-murzinov
Copy link
Author

Seems that this PR is useless since it's already implemented in #66

@ilya-murzinov
Copy link
Author

@dialelo I'll leave this PR open in case you'll find anything useful in it. But feel free to close it :)

@ghost ghost closed this Nov 2, 2016
This pull request was closed.
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.

2 participants