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

Magnolia related ideas #176

Open
nevillelyh opened this issue Oct 30, 2019 · 3 comments
Open

Magnolia related ideas #176

nevillelyh opened this issue Oct 30, 2019 · 3 comments
Assignees

Comments

@nevillelyh
Copy link
Contributor

https://github.com/spotify/scio/blob/master/build.sbt#L110

Scio master (since 0.8.0-beta1) has switched to latest magnolia (0.12.0) for Scala 2.13 while pinging my fork 0.10.1-jto for Scala 2.11 which is no longer supported. Mixing different magnolia versions has caused problem in the past.

I ported/cleaned up some common type classes into this new lib: https://github.com/spotify/magnolify

It'd be nice to consolidate various magnolia derivation code there.

  • case class generators
  • port shapeless based diffy -> magnolia-based (TBD)

WDYT?

@idreeskhan
Copy link
Contributor

idreeskhan commented Oct 31, 2019

Bit worried about further fragmenting our codebase. The case class generator I'm more flexible on since it's tiny and mostly used by us at the moment. We could port CaseClassDiffy from shapeless to magnolia but haven't seen a need to prioritize that work yet, and I think it makes sense for that to continue living in Ratatool co-located with other diffy jobs

@drobert
Copy link

drobert commented Nov 21, 2019

Quick notes as someone just getting started:

  • Scio master is indeed on magnolia 0.12.0 however -beta2 is on 0.11.0 . I don't think a released version of scio brings in 0.12.0
  • I can't find changelogs published for magnolia anywhere so it's unclear what this change implies
  • I'm trying to get started with Ratatool and Scio together, and things work find in intellij but fail in CI due to java.lang.NoSuchMethodError: magnolia.TypeName.<init> <some signature here>.

It would be extremely nice for those of us in the Scio community to have a version of ratatool that "plays nice". This toolkit feels like a borderline necessity for property-based testing with Beam in scala.

@anne-decusatis
Copy link
Contributor

@drobert the issue you're seeing around magnolia.TypeName.<init> is one I also saw when I had both Magnolia 0.10.1-jto and a newer version of Magnolia in the same classpath.
I think it may fix your problem to move CaseClassGenerator into magnolify and remove the Magnolia dependency from Ratatool - then you can import ratatool-scalacheck without having the magnolia version conflict between Ratatool and Scio. (and we can handle that version conflict, which will still be an issue only if you want to use CaseClassGenerator, by keeping magnolify in sync with scio...)
Thanks for commenting, this helps us prioritize!

@idreeskhan idreeskhan self-assigned this Dec 3, 2019
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

No branches or pull requests

4 participants