-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Support jetty-eeX-env.xml, as per jetty-eeX-web.xml #11752
Conversation
@@ -47,6 +47,8 @@ public class EnvConfiguration extends AbstractConfiguration | |||
private static final Logger LOG = LoggerFactory.getLogger(EnvConfiguration.class); | |||
|
|||
private static final String JETTY_ENV_BINDINGS = "org.eclipse.jetty.jndi.EnvConfiguration"; |
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.
this binding key isn't ee10 specific, should this be in core?
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.
Maybe but there's no good place to put it (yet?).
@@ -47,6 +47,8 @@ public class EnvConfiguration extends AbstractConfiguration | |||
private static final Logger LOG = LoggerFactory.getLogger(EnvConfiguration.class); | |||
|
|||
private static final String JETTY_ENV_BINDINGS = "org.eclipse.jetty.jndi.EnvConfiguration"; |
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.
this name isn't ee11 specific, shouldn't this be in jetty-core?
@@ -50,6 +49,8 @@ public class EnvConfiguration extends AbstractConfiguration | |||
private static final Logger LOG = LoggerFactory.getLogger(EnvConfiguration.class); | |||
|
|||
private static final String JETTY_ENV_BINDINGS = "org.eclipse.jetty.jndi.EnvConfiguration"; |
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.
This name isn't ee9 specific, shouldn't it be in jetty core?
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.
Minor nit... I found the xmlFile field confusing and was looking for more behaviour for it. I don't think we need it in the debug as the exception itself will be clear
...-ee10/jetty-ee10-plus/src/main/java/org/eclipse/jetty/ee10/plus/webapp/EnvConfiguration.java
Outdated
Show resolved
Hide resolved
...-ee11/jetty-ee11-plus/src/main/java/org/eclipse/jetty/ee11/plus/webapp/EnvConfiguration.java
Outdated
Show resolved
Hide resolved
jetty-ee9/jetty-ee9-plus/src/main/java/org/eclipse/jetty/ee9/plus/webapp/EnvConfiguration.java
Outdated
Show resolved
Hide resolved
…0/plus/webapp/EnvConfiguration.java Co-authored-by: Greg Wilkins <gregw@webtide.com>
…lus/webapp/EnvConfiguration.java Co-authored-by: Greg Wilkins <gregw@webtide.com>
…1/plus/webapp/EnvConfiguration.java Co-authored-by: Greg Wilkins <gregw@webtide.com>
I think this and the jetty-web.xml change should be backported to 12.0 |
* Support jetty-eeX-env.xml, as per jetty-eeX-web.xml --------- Co-authored-by: Greg Wilkins <gregw@webtide.com>
Need to support ee specific jetty-env.xml file, just as we do for jetty-web.xml