Switch over developer prepareCompose workflow to jetty #2941
Conversation
devinrsmith
force-pushed
the
developer-jetty-switch
branch
from
e84c4ed to
d462002
Compare
There was a problem hiding this comment.
Main feedback is that i think we might want to also rename/alias server to server-netty, and consider making server instead point to server-jetty (or leave it for one more version to avoid breaking anyone using deephaven/server and expecting netty). In theory there should be no difference, since the current server-jetty-app only does more than server-netty-app, but perhaps worth doing this work to be clear about the transition period?
|
The big noticable different is port 8080 vs port 10000. |
|
Instead of disabling some and leaving others, it was easiest to rename all of the relevant server images w/ the |
| into context | ||
| } | ||
|
|
||
| def buildDockerClosure = { String base, String server -> |
There was a problem hiding this comment.
The netty build has:
// TODO(deephaven-core#1596): Remove extra dependencies for build-ci.yml
// Currently, GH build-ci.yml calls prepareDockerAll, and relies on buildx; so we need to
// make sure that dockerCreateDockerfile has the proper dependencies.
// If we change how our build works in the future (so that gradle is responsible for pushing the
// image), we can remove this extra dependency.
project.tasks.register('prepareDockerAll') {
dependsOn prepareDocker, baseMap.keySet().collect { base -> Docker.registryTask(project, base) }
}
Why don't we need this, just because we're still using netty for CI?
There was a problem hiding this comment.
This was mostly a copy-paste from the netty logic; I'm not sure if this specific block is still necessary since we don't have an CI artifacts for docker + jetty in deephaven-core.
There are non-trivial dependencies beyond for other build related things that depend on server-netty. For example, sphinx. I'm betting these will be "easy" to fix over to server-jetty; but it doesn't serve the purpose of the release IMO so I tried to limit the scope as much as possible.
Technically, there are a bunch of folder renaming that I didn't need to do - but I did anyways, just to keep things clearer w/ the universal -netty suffix.
The build logic in server-jetty is meant to serve local development purposes for ./gradlew prepareCompose; docker compose up -d workflows.
| it.dependsOn project(':docker-server-slim').tasks.findByName('buildDocker-server-slim'), | ||
| project(':docker-server').tasks.findByName('buildDocker-server'), | ||
| project(':docker-web').tasks.findByName('buildDocker'), | ||
| project(':envoy').tasks.findByName('buildDocker'), |
There was a problem hiding this comment.
to confirm, CI will still build and publish these, right?
| @@ -45,7 +45,7 @@ public abstract class DeephavenInDockerExtension { | |||
| // irritating configuration order of operations to work out here, so just leaving | |||
| // these as constants until we decide they aren't any more | |||
| deephavenServerProject = ':docker-server' | |||
| serverTask = 'buildDocker-server' | |||
There was a problem hiding this comment.
we should switch over to jetty for this too, but happy to defer. sphinx also would be fine to move over, the fewer things leaning on netty the better.
i'll fill something to that effect.
devinrsmith
added
the
release blocker
Tested w/
./gradlew prepareCompose; docker compose up -d