Geronimo
  1. Geronimo
  2. GERONIMO-4742

Connector portlets for Tomcat does not work

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.2
    • Fix Version/s: 2.2
    • Component/s: None
    • Security Level: public (Regular issues)
    • Labels:
      None
    • Patch Info:
      Patch Available

      Description

      After refactoriing Tomcat integratoin codes, currently, the connector portlet does not work.

      1. G4742_B22.patch
        90 kB
        Shawn Jiang

        Activity

        Ivan created issue -
        Hide
        Ivan added a comment -

        I am thinking this issue for a while, here is my current ideas,
        In Geronimo 2.2, we use server.xml to hold all our configurations for Tomcat. While adding the new connector for the Tomcat, we have three ways to save it, The first one is to use ConnectorGBean, which is used in the past.
        Advantage: Less codes change
        Disadvantage : We have to maintain those configurations in two places
        The second way is to use server.xml, which means marshall the new configuration to server.xml file with JAXB.,
        Advantage: Only use server.xml to hold the configurations
        Disadvantage: Need to be care some issues, such as how to keep the placeholders in the server.xml ? We could not keep the comments in the server.xml. We need to be care about the codes, for we have two type assembilities: Jetty and Tomcat
        The last one is to use a big edit box, like what we do in ActiveMQ portlet
        Advantage: Easy,
        Disadvantage: The users get different experiences between Jetty and Tomcat.
        Personally, I will prefer to the second one and the third one.
        Any comment ? Thanks !

        Show
        Ivan added a comment - I am thinking this issue for a while, here is my current ideas, In Geronimo 2.2, we use server.xml to hold all our configurations for Tomcat. While adding the new connector for the Tomcat, we have three ways to save it, The first one is to use ConnectorGBean, which is used in the past. Advantage: Less codes change Disadvantage : We have to maintain those configurations in two places The second way is to use server.xml, which means marshall the new configuration to server.xml file with JAXB., Advantage: Only use server.xml to hold the configurations Disadvantage: Need to be care some issues, such as how to keep the placeholders in the server.xml ? We could not keep the comments in the server.xml. We need to be care about the codes, for we have two type assembilities: Jetty and Tomcat The last one is to use a big edit box, like what we do in ActiveMQ portlet Advantage: Easy, Disadvantage: The users get different experiences between Jetty and Tomcat. Personally, I will prefer to the second one and the third one. Any comment ? Thanks !
        Hide
        David Jencks added a comment -

        At this point in the 2.2 release cycle I like "easy". How about a big edit box together with samples for each connector that can be cut/pasted and editied?

        Show
        David Jencks added a comment - At this point in the 2.2 release cycle I like "easy". How about a big edit box together with samples for each connector that can be cut/pasted and editied?
        Hide
        Shawn Jiang added a comment -

        If using a big edit box, you give the user options to change any elements of server.xml. If the user updated Engine or service level elements of server.xml. because tomcat is not like actimemq which might need multiple broker and is more easy to reload in memory. We have to notify the user that he or she needs to restart the server to make the updated server.xml take into effect.

        It's ridiculous because if the user has to restart the server. Why didn't the user just stop the server and edit the server.xml with a familar text editor directly? So I strongly object the idea to add a big edit box for tomcat.

        Show
        Shawn Jiang added a comment - If using a big edit box, you give the user options to change any elements of server.xml. If the user updated Engine or service level elements of server.xml. because tomcat is not like actimemq which might need multiple broker and is more easy to reload in memory. We have to notify the user that he or she needs to restart the server to make the updated server.xml take into effect. It's ridiculous because if the user has to restart the server. Why didn't the user just stop the server and edit the server.xml with a familar text editor directly? So I strongly object the idea to add a big edit box for tomcat.
        Hide
        Shawn Jiang added a comment -

        If we don't choose big edit box, we are choosing to keep current port-let.

        Current web server port-let is used to provide a easy way to manage the connectors of web container. I really don't think this is useful as before because the connector configuration method have been changed to server.xml.

        1, Unlike GBean way, the server.xml way to configure connector is very easy and straightforward.
        2, IIUC, Web Connector configuration is not a very frequent task from my understanding.

        For the two reasons listed above, I highly doubt if it's worthy to add many wrap connectorGBeans for connectors loaded from server.xml just to meet a function that is not very useful anymore. What I'm suggesting is to remove the current web manager port-let for tomcat build directly. : )

        What do you say ?

        Show
        Shawn Jiang added a comment - If we don't choose big edit box, we are choosing to keep current port-let. Current web server port-let is used to provide a easy way to manage the connectors of web container. I really don't think this is useful as before because the connector configuration method have been changed to server.xml. 1, Unlike GBean way, the server.xml way to configure connector is very easy and straightforward. 2, IIUC, Web Connector configuration is not a very frequent task from my understanding. For the two reasons listed above, I highly doubt if it's worthy to add many wrap connectorGBeans for connectors loaded from server.xml just to meet a function that is not very useful anymore. What I'm suggesting is to remove the current web manager port-let for tomcat build directly. : ) What do you say ?
        Ivan made changes -
        Field Original Value New Value
        Assignee Ivan [ xuhaihong ] Shawn Jiang [ genspring ]
        Hide
        Donald Woods added a comment -

        Loss of Admin config functionality just to rush a release out the door is not acceptable in my book....
        We've always gotten good praise and reviews for our Admin console, so why go against that now?

        Show
        Donald Woods added a comment - Loss of Admin config functionality just to rush a release out the door is not acceptable in my book.... We've always gotten good praise and reviews for our Admin console, so why go against that now?
        Hide
        Shawn Jiang added a comment -

        Thanks Donald for your comments, I'm going to fix the connector portlet :

        1, convert all tomcat connector loaded in server.xml into GBeans when the server is starting up.

        2, The GBean won't be stored back to config.xml

        3, The new added GBean will be stored to server.xml.

        Some existing problem:

        1, There's no name and description of connectors defined in server.xml
        2, must use dom instead of jaxb to store connector back to server.xml.(in order to keep the comments and placeholders)

        Show
        Shawn Jiang added a comment - Thanks Donald for your comments, I'm going to fix the connector portlet : 1, convert all tomcat connector loaded in server.xml into GBeans when the server is starting up. 2, The GBean won't be stored back to config.xml 3, The new added GBean will be stored to server.xml. Some existing problem: 1, There's no name and description of connectors defined in server.xml 2, must use dom instead of jaxb to store connector back to server.xml.(in order to keep the comments and placeholders)
        Hide
        Shawn Jiang added a comment -

        This is the patch for Branch 2.2. Please review. I'm verifying the patch on trunk, I believe the patch for trunk should be the same.

        Show
        Shawn Jiang added a comment - This is the patch for Branch 2.2. Please review. I'm verifying the patch on trunk, I believe the patch for trunk should be the same.
        Shawn Jiang made changes -
        Attachment G4742_B22.patch [ 12417435 ]
        Shawn Jiang made changes -
        Patch Info [Patch Available]
        Hide
        Shawn Jiang added a comment -

        If no objection, I'll commit the patch to 22 branch and trunk. Thanks.

        Show
        Shawn Jiang added a comment - If no objection, I'll commit the patch to 22 branch and trunk. Thanks.
        Hide
        Shawn Jiang added a comment -

        Committed the patch to 22 branch and trunk:

        1, Added ConnectorWrapperGBeanStarter.java to start connector GBeans for connectors defined in server.xml.
        2, Added TomcatServerConfigManager.java to use DOM to read/write connectors from server.xml.
        3, Modified other classes to make this happen.

        Show
        Shawn Jiang added a comment - Committed the patch to 22 branch and trunk: 1, Added ConnectorWrapperGBeanStarter.java to start connector GBeans for connectors defined in server.xml. 2, Added TomcatServerConfigManager.java to use DOM to read/write connectors from server.xml. 3, Modified other classes to make this happen.
        Hide
        Shawn Jiang added a comment -

        Resolving it.

        Show
        Shawn Jiang added a comment - Resolving it.
        Shawn Jiang made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Shawn Jiang added a comment -

        Closing it.

        Show
        Shawn Jiang added a comment - Closing it.
        Shawn Jiang made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

          People

          • Assignee:
            Shawn Jiang
            Reporter:
            Ivan
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development