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

Use sbt's multi-jvm instead of our manual implementation? #548

Open
Tracked by #1052
mdedetrich opened this issue Aug 8, 2023 · 6 comments
Open
Tracked by #1052

Use sbt's multi-jvm instead of our manual implementation? #548

mdedetrich opened this issue Aug 8, 2023 · 6 comments

Comments

@mdedetrich
Copy link
Contributor

As a result of the discussion at apache/pekko-http#297 (comment) I just found out that there is already a sbt-multi-jvm plugin that appears to be identical to ours and was originally upstream by Akka/Akka community, presumably for Akka to eventually remove the multi jvm setup in their builds which we inherited.

I think we should seriously consider upstreaming any necessary changes and then using the plugin, especially considering that we now have duplicated code between pekko and pekko-http.

@jrudolph @pjfanning @He-Pin @raboof wdyt ?

@mdedetrich
Copy link
Contributor Author

From apache/pekko-http#297 (comment) @He-Pin

We need wait the Controller transited to connectable before start the Player.

I think this concept is easily generalizable given that its not a problem thats specific to Pekko (after testing clusters is the reason we are using multiple JVM's in the first place)

Need submit a PR to sbt-multi-jvm and waiting a new snapshot, which may take sometimes and is not a quick fix.

This is a non issue at least in regards to using sbt-multi-jvm right now. I am thinking about this long term, but typically if its seen there are significant contributions made to a project than you get added as a maintainer (these are community projects after all).

I did not diff the code with sbt-multi-jvm line by line, but there must be some reason for Akka was keep it separately

From what I could see by quickly glancing at the code, the intention was to replace sbt-multi-jvm withe code in Akka but they never got around to it likely because of priority/effort required.

@He-Pin
Copy link
Member

He-Pin commented Aug 8, 2023

  1. That's was a quick fix so I duplicated the code to keep night build works.
  2. For future, we should extract all shared plugins to something as what you once suggested and typelevel does, eg: apache-sbt-pekko?

@mdedetrich
Copy link
Contributor Author

mdedetrich commented Aug 8, 2023

That's was a quick fix so I duplicated the code to keep night build runing.

Understood and fine for now

For future, we should extract all shared plugins to something as what you once suggested and typelevel does, eg: apache-sbt-pekko?

This makes sense for features that are unique to pekko, i.e. think our Paradox theme or setting up the organization of our projects to org.apache.pekko. multi-jvm however is just a general feature so it makes more sense if its possible to use sbt-multi-jvm.

This isn't any different to sbt-license-report, which our Pekko projects use but is an sbt community plugin (see https://github.com/sbt/sbt-license-report) and if you look at https://github.com/sbt/sbt-license-report/pulls?q=is%3Apr+author%3Amdedetrich and https://github.com/sbt/sbt-license-report/releases you can see that contributing upsream and getting a release wasn't an issue at all.

As I said before, generally speaking if its shown that you contribute features upstream there isn't usually a problem in those releases being made.

@He-Pin
Copy link
Member

He-Pin commented Aug 8, 2023

The sbt-multi-jvm can go upstream and you saw there are so many backlog there too.

@mdedetrich
Copy link
Contributor Author

mdedetrich commented Aug 8, 2023

The sbt-multi-jvm can go upstream and you saw there are so many backlog there too.

If you mean by issues then yes there does seem to be quite a few open https://github.com/sbt/sbt-multi-jvm/issues but there aren't any PR's for it. And with issues such as sbt/sbt-multi-jvm#36 this just strengthens my earlier assumption which is that Akka was upstreaming the sbt-multi-jvm functionality but didn't get around to finishing it.

I think that if we start opening up PR's to actually solve the issues mentioned before then that should get things going.

@He-Pin
Copy link
Member

He-Pin commented Aug 8, 2023

I think I can submit a PR which do not need the upstream change, @mdedetrich , which will do another round of refractory to the current Player impl. @mdedetrich .

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

2 participants