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

Improvements to ZQuery #160

Merged
merged 4 commits into from
Jan 11, 2020
Merged

Improvements to ZQuery #160

merged 4 commits into from
Jan 11, 2020

Conversation

adamgfraser
Copy link
Contributor

Various improvements to ZQuery including:

  • Adds new data source constructors fromFunctionBatchedWith and fromFunctionBatchedWithM that make it easier to construct batched data sources that don't return results in the same order as requests by providing a function to associate results with requests as suggested by @fokot.
  • Adds partitionM and partitionMPar methods on ZQuery that allow for performing a collection of queries and getting back a collection of all successes along with a collection of all failures
  • Adds a new Described data type to clean up and clarify the contract around needing to provide names for certain combinators such as provide and provideSome.
  • Miscellaneous cleanup including adding CanFail and NeedsEnv to appropriate combinators and removing unnecessary final modifiers.

One thing I would appreciate feedback on is how much users are working with data sources in the environment. I am thinking we could simplify DataSource somewhat by not using the module pattern and just having all the methods on DataSource versus DataSource.Service. That would mean that users could still put the DataSource in the environment but it would not come "baked in" like it is today. The nice thing would be this would mean you wouldn't have to write DataSource.Service everywhere but if people are using it as an environmental effect then we should leave as is.

@adamgfraser adamgfraser requested a review from ghostdogpr January 7, 2020 01:12
Copy link
Owner

@ghostdogpr ghostdogpr left a comment

Choose a reason for hiding this comment

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

About your question about DataSource, I personally prefer not to use ZIO Environment for that and pass it explicitly (like I did in OptimizedTest), so I support the simplification. We could also make fromRequestWith be just fromRequest. I'll point to your PR in discord to see if others have an opinion on it.

* `Either`. The resulting query cannot fail, because the failure case has
* been exposed as part of the `Either` success case.
*/
final def either(implicit ev: CanFail[E]): ZQuery[R, Nothing, Either[E, A]] =
Copy link
Owner

Choose a reason for hiding this comment

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

Nice! It can be used in Executor line 145

@jdegoes
Copy link

jdegoes commented Jan 7, 2020

Agreed on not using environment for DataSource. It's better an ordinary value.

ZIO Config used environment for its "ConfigSource" for a while, but changing that to values made everything much more natural and compositional, as well as more obvious for users.

@adamgfraser
Copy link
Contributor Author

Updated to not use the environment for DataSource and add additional error handling combinators foldM and bimap.

Copy link
Owner

@ghostdogpr ghostdogpr left a comment

Choose a reason for hiding this comment

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

Awesome!

@ghostdogpr ghostdogpr merged commit 2875dd0 into ghostdogpr:master Jan 11, 2020
@adamgfraser adamgfraser deleted the zquery branch January 14, 2020 13:19
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