Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Issue #10437 - Unify Deployer ContextProvider #12583
base: jetty-12.1.x
Are you sure you want to change the base?
Issue #10437 - Unify Deployer ContextProvider #12583
Changes from 26 commits
8878567
3a46e37
c5c07c2
e0bdeb3
7588d5f
42015b3
f9c90ce
83fc5f1
a0c9058
249679f
bef04d9
add337a
2e9660d
9bfab80
2c651ea
0e534f7
43eb679
c5a00b8
229e358
a363189
5f6d504
77c34c2
5306c3d
96d5bec
ee18845
e498f2c
2b0741b
9d9267d
cf69f26
4fd255b
1ebdd10
d5def4c
54960bd
c16fdf7
bc1b50f
24eb791
4d286d7
40052bd
6806b58
8f32176
f92901d
c7fdc92
745f4fe
456132f
dfd741b
72c7304
fbedd51
4bb0850
1e29659
2acf58e
7b5cd4e
318ab9e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this interface is not named exactly accurately, and the class javadoc is misleading. According to its api, it isn't creating or providing any
App
instances to theDeploymentManager
at all. Looking at it's api, it's function is really to create aContextHandler
instance - after some other class passes in an already created instance ofApp
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unit
a way thatScanningAppProvider
tracks its collection ofPath
objects to anApp
via it's basename. (as long as the basename exists, this Unit exist)App
the fundamental unit used to track theApp
through the deployment lifecycle. (created as part of new deployment, used as reference for moving through lifecycle: undeploy/remove steps)ContextHandler
the feature of the Jetty Server that represents the instantiated and liveApp
While all of these seem like the same "thing" to track as a single place, it is really 3 levels of abstraction, each with its own life cycle. The
Unit
has the longest lifecycle, followed byApp
with a shorter lifecycle, and finallyContextHandler
with the shortest (from the point of view of a deployment manager).A
Unit
exists for as long as there arePaths
with a basename being tracked. (only lives inScanningAppProvider
)An
App
only exists if there is anAppProvider
that creates it. (note thatScanningAppProvider
can have aUnit
with noApp
if there is no main deployment path present for thatUnit
)A
ContextHandler
only exists if there is a need for it in the lifecycle binding. (If a step in the lifecycle needs aContextHandler
then that's when it gets created or accessed. Our standard lifecycle bindings will use the ContextHandler for deploy/start/stop/undeploy, but not the other bindings)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
ContextHandler
can even change as theApp
moves through the registered lifecycle bindings.For example, a custom binding could wrap the
ContextHandler
to add an auditing layer.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The lifecycle relationships are such that neither an
App
nor aContextHandler
(in the sense of deployer) can exist without aUnit
. Sure,ContextHandler
andApp
have their own lifecycles, but they ultimately depend on the existence or otherwise of theUnit
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ContextHandlerFactory would be a better name for this interface
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another feature of
Unit
is that it represents a group of paths that influence the deployment process.One aspect not made clear is that the environment configuration files (
webapps/<env>.xml
,webapps/<env>-<name>.xml
,webapps/<env>.properties
, andwebapps/<env>-<name>.properties
) are also considered aUnit
(with basename<env>
) that is tracked by the Scanner. (this is a feature that's been in Jetty 12.0.x since its inception). If one of those files in the unit are changed, then that triggers all associatedApp
s on that environment to also hot-redeploy.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Getting a good simple description of what a Unit is would be good. Then perhaps a better name would come from that.