Skip to content
This repository was archived by the owner on Jun 2, 2025. It is now read-only.

Conversation

@aevanszen
Copy link

As discussed in https://github.com/disneystreaming/weaver-test/issues/479.

This is a minimal implmentation to allow GlobalResources to be reused in a submodule / external jar across projects.

I've tested on a local project and it looks to work as expected.

This is useful if you want to package GlobalResource up as a subproject and re-use in a multi-module build.
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.


private def spiGlobalResources: List[GlobalResourceF[F]] = {
val result = ListBuffer.empty[GlobalResourceF[F]]
val loader: ServiceLoader[GlobalResourceF[F]] = ServiceLoader.load(classOf[GlobalResourceF[F]])
Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @aevanszen, thanks for the PR.

I gave it some long thought, and the problem with doing it this way is type-erasure. Even if it's unlikely that people would use different effect types in a single codebase, I'd be keen to protect against it. Because GlobalResourceF is parameterised, the load call would return everything, whether it's using cats-effect, monix or zio, and obviously loading monix globals in a CE context wouldn't be great.

So what I think we should do instead, would be for the ServiceLoader call to be abstract away. We have a SuiteLoader abstraction, and we could add a def loadGlobalResources to it.

The implementation of SuiteLoader lives there, at a place that is aware of the classes that the test-framework is supposed to understand. Those are effect-type specific (there is one set defined for CE, one for monix, one for zio etc).

Now the thing is that you'll need one other layer of indirection to support cross-compilation. IE you could have a PlatformServiceLoader object defined in a src-jvm, containing a spiGlobalResources[T](cl : ClassLoader, cls: Class[T]) : List[T] method (similar to what you've implemented here), and a similar object defined in src-js , that'd have a method with the same signature but would return an empty list.

This way, type-erasure would be accounted for.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for the detailed reply @Baccata, the suggestions are very helpful.

I had the same reservations around type-erasure but wasn't sure the best way to work around it in the framework just yet. Your suggestion is a good starting point, and I'll look into it.

I'll add the tests and documentation when the implementation is ok and update the PR with the changes.

The GlobalResoure classpath isn't an urgent blocker for me, so updating this PR may be slow as I work on it when I get the spare time. I will try to work on it, though, as it'll be useful to have.

Copy link
Contributor

Choose a reason for hiding this comment

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

No problem whatsoever, I certainly appreciate the contribution, even if it takes time. There is no rush :) let us know if you get stuck or if you want us to have a look at some commits

@Baccata
Copy link
Contributor

Baccata commented Mar 7, 2022

I'd also appreciate :

  • a test (though I don't mind if we defer, unless you want to take a stab at it).
  • an amendment to the documentation page to describe the SPI-based mechanism

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants