You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by "Reamer (via GitHub)" <gi...@apache.org> on 2023/06/19 17:55:17 UTC
[GitHub] [zeppelin] Reamer opened a new pull request, #4621: [ZEPPELIN-5933] Polish jetty
Reamer opened a new pull request, #4621:
URL: https://github.com/apache/zeppelin/pull/4621
### What is this PR for?
This PR addresses all comments from the following comment. https://github.com/eclipse/jetty.project/issues/6592#issuecomment-897877018
The comments were very helpful to do a better configuration in the code.
Especially `server.setDumpAfterStart(true);` helped to get a good overview. Since the dump is quite long and only helpful for debugging, I added it temporarily during development. However, due to its great usefulness, the command should not go unmentioned.
During development, I made sure that we did not use any special Jersey functions to simplify possible portability.
```
packages("org.apache.zeppelin.rest");
```
Since I forgot to include the Session API during development, I added a simple test class.
Please note that we currently expose the API and various other endpoints (e.g. Prometheus, Ping, Health) on two endpoints with appropriate configuration.
For the API endpoint once under `/api/` and once under `/next/api/`.
I don't think this will cause any problems.
### What type of PR is it?
* Improvement
### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-5933
### How should this be tested?
* CI
### Questions:
* Does the license files need to update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] Reamer commented on pull request #4621: [ZEPPELIN-5933] Polish jetty
Posted by "Reamer (via GitHub)" <gi...@apache.org>.
Reamer commented on PR #4621:
URL: https://github.com/apache/zeppelin/pull/4621#issuecomment-1608925309
> is `web.xml` needed?
I think so.
I don't think the context is currently being used anywhere.
```
<context-param>
<param-name>configuration</param-name>
<param-value>deployment</param-value>
</context-param>
```
but the session-config should work.
```
<session-config>
<cookie-config>
<http-only>true</http-only>
<secure>true</secure>
</cookie-config>
</session-config>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] jongyoul commented on a diff in pull request #4621: [ZEPPELIN-5933] Polish jetty
Posted by "jongyoul (via GitHub)" <gi...@apache.org>.
jongyoul commented on code in PR #4621:
URL: https://github.com/apache/zeppelin/pull/4621#discussion_r1236471742
##########
zeppelin-server/src/main/java/org/apache/zeppelin/server/RestApiApplication.java:
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.zeppelin.server;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import javax.ws.rs.core.Application;
+
+import org.apache.zeppelin.rest.AdminRestApi;
+import org.apache.zeppelin.rest.ClusterRestApi;
+import org.apache.zeppelin.rest.ConfigurationsRestApi;
+import org.apache.zeppelin.rest.CredentialRestApi;
+import org.apache.zeppelin.rest.HeliumRestApi;
+import org.apache.zeppelin.rest.InterpreterRestApi;
+import org.apache.zeppelin.rest.LoginRestApi;
+import org.apache.zeppelin.rest.NotebookRepoRestApi;
+import org.apache.zeppelin.rest.NotebookRestApi;
+import org.apache.zeppelin.rest.SecurityRestApi;
+import org.apache.zeppelin.rest.SessionRestApi;
+import org.apache.zeppelin.rest.ZeppelinRestApi;
+import org.apache.zeppelin.rest.exception.WebApplicationExceptionMapper;
+
+public class RestApiApplication extends Application {
+ @Override
+ public Set<Class<?>> getClasses() {
Review Comment:
Basically, we agreed to keep standards. It's a bit hard to improve Zeppelin continuously. I believe that the most important part of Zeppelin is providing our logic - like notebook feature and visualization -, not to try to keep the web standard stack. When we have a lot of contributors, it was ok to keep both of them but I, recently, feel like it's time to focus on what we will go for. It would be a long-term discussion and it doesn't mean that we should change rapidly to another framework or adopt a new tech. Keeping standards will, definitely, help to improve Zeppelin eventually. I just would like to talk to you broadly and freely. :-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] jongyoul commented on a diff in pull request #4621: [ZEPPELIN-5933] Polish jetty
Posted by "jongyoul (via GitHub)" <gi...@apache.org>.
jongyoul commented on code in PR #4621:
URL: https://github.com/apache/zeppelin/pull/4621#discussion_r1236475777
##########
zeppelin-server/src/main/java/org/apache/zeppelin/server/RestApiApplication.java:
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.zeppelin.server;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import javax.ws.rs.core.Application;
+
+import org.apache.zeppelin.rest.AdminRestApi;
+import org.apache.zeppelin.rest.ClusterRestApi;
+import org.apache.zeppelin.rest.ConfigurationsRestApi;
+import org.apache.zeppelin.rest.CredentialRestApi;
+import org.apache.zeppelin.rest.HeliumRestApi;
+import org.apache.zeppelin.rest.InterpreterRestApi;
+import org.apache.zeppelin.rest.LoginRestApi;
+import org.apache.zeppelin.rest.NotebookRepoRestApi;
+import org.apache.zeppelin.rest.NotebookRestApi;
+import org.apache.zeppelin.rest.SecurityRestApi;
+import org.apache.zeppelin.rest.SessionRestApi;
+import org.apache.zeppelin.rest.ZeppelinRestApi;
+import org.apache.zeppelin.rest.exception.WebApplicationExceptionMapper;
+
+public class RestApiApplication extends Application {
+ @Override
+ public Set<Class<?>> getClasses() {
Review Comment:
Moreover, the reason why I'm talking about Spring is that we will reduce our code by using Spring's predefined functions. It should be helpful when we verify our logic. I think we are now developing many things by ourselves. As a developer, I also like it personally.
I, myself, feel like I'm a bit biased so please correct me if I went so far.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] Reamer commented on a diff in pull request #4621: [ZEPPELIN-5933] Polish jetty
Posted by "Reamer (via GitHub)" <gi...@apache.org>.
Reamer commented on code in PR #4621:
URL: https://github.com/apache/zeppelin/pull/4621#discussion_r1237088889
##########
zeppelin-server/src/main/java/org/apache/zeppelin/server/RestApiApplication.java:
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.zeppelin.server;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import javax.ws.rs.core.Application;
+
+import org.apache.zeppelin.rest.AdminRestApi;
+import org.apache.zeppelin.rest.ClusterRestApi;
+import org.apache.zeppelin.rest.ConfigurationsRestApi;
+import org.apache.zeppelin.rest.CredentialRestApi;
+import org.apache.zeppelin.rest.HeliumRestApi;
+import org.apache.zeppelin.rest.InterpreterRestApi;
+import org.apache.zeppelin.rest.LoginRestApi;
+import org.apache.zeppelin.rest.NotebookRepoRestApi;
+import org.apache.zeppelin.rest.NotebookRestApi;
+import org.apache.zeppelin.rest.SecurityRestApi;
+import org.apache.zeppelin.rest.SessionRestApi;
+import org.apache.zeppelin.rest.ZeppelinRestApi;
+import org.apache.zeppelin.rest.exception.WebApplicationExceptionMapper;
+
+public class RestApiApplication extends Application {
+ @Override
+ public Set<Class<?>> getClasses() {
Review Comment:
If it goes towards Spring and it makes the code simpler and maybe faster, I don't mind. So long I will stick to the current structure and try to update the components.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] Reamer commented on a diff in pull request #4621: [ZEPPELIN-5933] Polish jetty
Posted by "Reamer (via GitHub)" <gi...@apache.org>.
Reamer commented on code in PR #4621:
URL: https://github.com/apache/zeppelin/pull/4621#discussion_r1235380362
##########
zeppelin-server/src/main/java/org/apache/zeppelin/server/RestApiApplication.java:
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.zeppelin.server;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import javax.ws.rs.core.Application;
+
+import org.apache.zeppelin.rest.AdminRestApi;
+import org.apache.zeppelin.rest.ClusterRestApi;
+import org.apache.zeppelin.rest.ConfigurationsRestApi;
+import org.apache.zeppelin.rest.CredentialRestApi;
+import org.apache.zeppelin.rest.HeliumRestApi;
+import org.apache.zeppelin.rest.InterpreterRestApi;
+import org.apache.zeppelin.rest.LoginRestApi;
+import org.apache.zeppelin.rest.NotebookRepoRestApi;
+import org.apache.zeppelin.rest.NotebookRestApi;
+import org.apache.zeppelin.rest.SecurityRestApi;
+import org.apache.zeppelin.rest.SessionRestApi;
+import org.apache.zeppelin.rest.ZeppelinRestApi;
+import org.apache.zeppelin.rest.exception.WebApplicationExceptionMapper;
+
+public class RestApiApplication extends Application {
+ @Override
+ public Set<Class<?>> getClasses() {
Review Comment:
The scanning process is only possible via the special function of Jersey.
```
<init-param>
<param-name>jersey.config.server.provider.packages</param-name>
<param-value>org.apache.zeppelin.rest</param-value>
</init-param>
```
See method one.
https://stackoverflow.com/questions/22994690/which-init-param-to-use-jersey-config-server-provider-packages-or-javax-ws-rs-a
To simplify later migrations, I decided to use method two, where unfortunately you have to list all classes.
In my opinion, the additional writing effort is kept within limits.
A way over annotations is not known to me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] Reamer commented on a diff in pull request #4621: [ZEPPELIN-5933] Polish jetty
Posted by "Reamer (via GitHub)" <gi...@apache.org>.
Reamer commented on code in PR #4621:
URL: https://github.com/apache/zeppelin/pull/4621#discussion_r1235380362
##########
zeppelin-server/src/main/java/org/apache/zeppelin/server/RestApiApplication.java:
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.zeppelin.server;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import javax.ws.rs.core.Application;
+
+import org.apache.zeppelin.rest.AdminRestApi;
+import org.apache.zeppelin.rest.ClusterRestApi;
+import org.apache.zeppelin.rest.ConfigurationsRestApi;
+import org.apache.zeppelin.rest.CredentialRestApi;
+import org.apache.zeppelin.rest.HeliumRestApi;
+import org.apache.zeppelin.rest.InterpreterRestApi;
+import org.apache.zeppelin.rest.LoginRestApi;
+import org.apache.zeppelin.rest.NotebookRepoRestApi;
+import org.apache.zeppelin.rest.NotebookRestApi;
+import org.apache.zeppelin.rest.SecurityRestApi;
+import org.apache.zeppelin.rest.SessionRestApi;
+import org.apache.zeppelin.rest.ZeppelinRestApi;
+import org.apache.zeppelin.rest.exception.WebApplicationExceptionMapper;
+
+public class RestApiApplication extends Application {
+ @Override
+ public Set<Class<?>> getClasses() {
Review Comment:
The scanning process is only possible via the special function of Jersey.
```
<init-param>
<param-name>jersey.config.server.provider.packages</param-name>
<param-value>org.apache.zeppelin.rest</param-value>
</init-param>
```
See method one.
https://stackoverflow.com/questions/22994690/which-init-param-to-use-jersey-config-server-provider-packages-or-javax-ws-rs-a
To simplify later migrations, I decided to use method two, where unfortunately you have to list all classes.
In my opinion, the additional writing effort is kept within limits.
A way that uses only annotations is not known to me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] jongyoul commented on a diff in pull request #4621: [ZEPPELIN-5933] Polish jetty
Posted by "jongyoul (via GitHub)" <gi...@apache.org>.
jongyoul commented on code in PR #4621:
URL: https://github.com/apache/zeppelin/pull/4621#discussion_r1235968470
##########
zeppelin-server/src/main/java/org/apache/zeppelin/server/RestApiApplication.java:
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.zeppelin.server;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import javax.ws.rs.core.Application;
+
+import org.apache.zeppelin.rest.AdminRestApi;
+import org.apache.zeppelin.rest.ClusterRestApi;
+import org.apache.zeppelin.rest.ConfigurationsRestApi;
+import org.apache.zeppelin.rest.CredentialRestApi;
+import org.apache.zeppelin.rest.HeliumRestApi;
+import org.apache.zeppelin.rest.InterpreterRestApi;
+import org.apache.zeppelin.rest.LoginRestApi;
+import org.apache.zeppelin.rest.NotebookRepoRestApi;
+import org.apache.zeppelin.rest.NotebookRestApi;
+import org.apache.zeppelin.rest.SecurityRestApi;
+import org.apache.zeppelin.rest.SessionRestApi;
+import org.apache.zeppelin.rest.ZeppelinRestApi;
+import org.apache.zeppelin.rest.exception.WebApplicationExceptionMapper;
+
+public class RestApiApplication extends Application {
+ @Override
+ public Set<Class<?>> getClasses() {
Review Comment:
Thanks. By the way, I changed the code from `Application` to `ResourceConfig` before but I agree with you that it's better to avoid jersey's dependency.
I have a question about the future plan. What do you think we should go further instead of using jersey? I've tried to adopt Spring framework last year with some contributors but it was not that easy at that time. If we would like to do, we should be careful and change gradually. I, however, really like to improve our current structure.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] Reamer commented on a diff in pull request #4621: [ZEPPELIN-5933] Polish jetty
Posted by "Reamer (via GitHub)" <gi...@apache.org>.
Reamer commented on code in PR #4621:
URL: https://github.com/apache/zeppelin/pull/4621#discussion_r1236422121
##########
zeppelin-server/src/main/java/org/apache/zeppelin/server/RestApiApplication.java:
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.zeppelin.server;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import javax.ws.rs.core.Application;
+
+import org.apache.zeppelin.rest.AdminRestApi;
+import org.apache.zeppelin.rest.ClusterRestApi;
+import org.apache.zeppelin.rest.ConfigurationsRestApi;
+import org.apache.zeppelin.rest.CredentialRestApi;
+import org.apache.zeppelin.rest.HeliumRestApi;
+import org.apache.zeppelin.rest.InterpreterRestApi;
+import org.apache.zeppelin.rest.LoginRestApi;
+import org.apache.zeppelin.rest.NotebookRepoRestApi;
+import org.apache.zeppelin.rest.NotebookRestApi;
+import org.apache.zeppelin.rest.SecurityRestApi;
+import org.apache.zeppelin.rest.SessionRestApi;
+import org.apache.zeppelin.rest.ZeppelinRestApi;
+import org.apache.zeppelin.rest.exception.WebApplicationExceptionMapper;
+
+public class RestApiApplication extends Application {
+ @Override
+ public Set<Class<?>> getClasses() {
Review Comment:
I think we should stick to general standards if possible, so that developers looking at Zeppelin from a different Java stack can get a quick overview.
"Magic" functions often involve less code, but require expertise.
Currently, I have no problem with jersey. There is a continuous development. Possibly more popular frameworks such as Spring can help make the Zeppelin stack clear to a broader developer community, but again I'd like to focus on standards.
Spring today, Quarkus tomorrow, and then something else.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] Reamer merged pull request #4621: [ZEPPELIN-5933] Polish jetty
Posted by "Reamer (via GitHub)" <gi...@apache.org>.
Reamer merged PR #4621:
URL: https://github.com/apache/zeppelin/pull/4621
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] Reamer commented on pull request #4621: [ZEPPELIN-5933] Polish jetty
Posted by "Reamer (via GitHub)" <gi...@apache.org>.
Reamer commented on PR #4621:
URL: https://github.com/apache/zeppelin/pull/4621#issuecomment-1597553878
Hi @joakime
I hope you are still active in the Jetty project and wanted to take this opportunity to thank you for your detailed https://github.com/eclipse/jetty.project/issues/6592#issuecomment-897877018. I would be very grateful if you also share your opinion on the pull request.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] jongyoul commented on a diff in pull request #4621: [ZEPPELIN-5933] Polish jetty
Posted by "jongyoul (via GitHub)" <gi...@apache.org>.
jongyoul commented on code in PR #4621:
URL: https://github.com/apache/zeppelin/pull/4621#discussion_r1236471742
##########
zeppelin-server/src/main/java/org/apache/zeppelin/server/RestApiApplication.java:
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.zeppelin.server;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import javax.ws.rs.core.Application;
+
+import org.apache.zeppelin.rest.AdminRestApi;
+import org.apache.zeppelin.rest.ClusterRestApi;
+import org.apache.zeppelin.rest.ConfigurationsRestApi;
+import org.apache.zeppelin.rest.CredentialRestApi;
+import org.apache.zeppelin.rest.HeliumRestApi;
+import org.apache.zeppelin.rest.InterpreterRestApi;
+import org.apache.zeppelin.rest.LoginRestApi;
+import org.apache.zeppelin.rest.NotebookRepoRestApi;
+import org.apache.zeppelin.rest.NotebookRestApi;
+import org.apache.zeppelin.rest.SecurityRestApi;
+import org.apache.zeppelin.rest.SessionRestApi;
+import org.apache.zeppelin.rest.ZeppelinRestApi;
+import org.apache.zeppelin.rest.exception.WebApplicationExceptionMapper;
+
+public class RestApiApplication extends Application {
+ @Override
+ public Set<Class<?>> getClasses() {
Review Comment:
Basically, we agreed to keep standards. When we have a lot of contributors, it was ok to keep both of them but I, recently, feel like it's time to focus on what we will go for. It would be a long-term discussion and it doesn't mean that we should change rapidly to another framework or adopt a new tech. Keeping standards will, definitely, help to improve Zeppelin eventually. I just would like to talk to you broadly and freely. :-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] Reamer commented on a diff in pull request #4621: [ZEPPELIN-5933] Polish jetty
Posted by "Reamer (via GitHub)" <gi...@apache.org>.
Reamer commented on code in PR #4621:
URL: https://github.com/apache/zeppelin/pull/4621#discussion_r1235380362
##########
zeppelin-server/src/main/java/org/apache/zeppelin/server/RestApiApplication.java:
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.zeppelin.server;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import javax.ws.rs.core.Application;
+
+import org.apache.zeppelin.rest.AdminRestApi;
+import org.apache.zeppelin.rest.ClusterRestApi;
+import org.apache.zeppelin.rest.ConfigurationsRestApi;
+import org.apache.zeppelin.rest.CredentialRestApi;
+import org.apache.zeppelin.rest.HeliumRestApi;
+import org.apache.zeppelin.rest.InterpreterRestApi;
+import org.apache.zeppelin.rest.LoginRestApi;
+import org.apache.zeppelin.rest.NotebookRepoRestApi;
+import org.apache.zeppelin.rest.NotebookRestApi;
+import org.apache.zeppelin.rest.SecurityRestApi;
+import org.apache.zeppelin.rest.SessionRestApi;
+import org.apache.zeppelin.rest.ZeppelinRestApi;
+import org.apache.zeppelin.rest.exception.WebApplicationExceptionMapper;
+
+public class RestApiApplication extends Application {
+ @Override
+ public Set<Class<?>> getClasses() {
Review Comment:
The scanning process is only possible via the special function of Jersey.
```
<init-param>
<param-name>jersey.config.server.provider.packages</param-name>
<param-value>org.apache.zeppelin.rest</param-value>
</init-param>
```
See method one.
https://stackoverflow.com/questions/22994690/which-init-param-to-use-jersey-config-server-provider-packages-or-javax-ws-rs-a
To simplify later migrations, I decided to use method two, where unfortunately you have to list all classes.
In my opinion, the additional writing effort is kept within limits.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] jongyoul commented on a diff in pull request #4621: [ZEPPELIN-5933] Polish jetty
Posted by "jongyoul (via GitHub)" <gi...@apache.org>.
jongyoul commented on code in PR #4621:
URL: https://github.com/apache/zeppelin/pull/4621#discussion_r1235074369
##########
zeppelin-server/src/main/java/org/apache/zeppelin/server/RestApiApplication.java:
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.zeppelin.server;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import javax.ws.rs.core.Application;
+
+import org.apache.zeppelin.rest.AdminRestApi;
+import org.apache.zeppelin.rest.ClusterRestApi;
+import org.apache.zeppelin.rest.ConfigurationsRestApi;
+import org.apache.zeppelin.rest.CredentialRestApi;
+import org.apache.zeppelin.rest.HeliumRestApi;
+import org.apache.zeppelin.rest.InterpreterRestApi;
+import org.apache.zeppelin.rest.LoginRestApi;
+import org.apache.zeppelin.rest.NotebookRepoRestApi;
+import org.apache.zeppelin.rest.NotebookRestApi;
+import org.apache.zeppelin.rest.SecurityRestApi;
+import org.apache.zeppelin.rest.SessionRestApi;
+import org.apache.zeppelin.rest.ZeppelinRestApi;
+import org.apache.zeppelin.rest.exception.WebApplicationExceptionMapper;
+
+public class RestApiApplication extends Application {
+ @Override
+ public Set<Class<?>> getClasses() {
Review Comment:
I'm wondering if it should add `...RestApi` classes because it should work with annotations in my understanding. Is there any hint why we should them?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org