glassfish
  1. glassfish
  2. GLASSFISH-10200

create-network-listener: validate that transport and thread pool exist if specified in options

    Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: V3
    • Fix Version/s: future release
    • Component/s: admin
    • Labels:
      None
    • Environment:

      Operating System: All
      Platform: PC

    • Issuezilla Id:
      10,200
    • Status Whiteboard:
      Hide

      v3_exclude

      Show
      v3_exclude

      Description

      When the network listener is created with --transport mytcp and --threadpool
      mypool it is simply referenced when the network listener is created. The user
      has to create the transport and threadpool separately before he uses the listener .

      Why not create the transport and threadpool if it doesn't exist in the
      domain.xml and reference it, which will make the life easier for a developer who
      wish to create a listener without bothering about so many other commands.

      Ideally we should be doing this.

      1. If the option is not specified use the default values
      2. if the option is specified and and elements already exists in domain.xml
      reference it
      3. If the option is specified and the elements doesnot exists in domain.xml
      create new transport/threadpool's elements and reference it.

        Activity

        Hide
        Justin Lee added a comment -

        create-http-listener does, in fact, do this to maintain funcationality of v2.
        create-network-listener does not as this is consistent with other creates in
        that it creates only the one element. I'm not opposed to doing this but if we
        go this route, we might as well have create-network-listener just be an alias
        for create-http-listener. And that's ok with me, too.

        I'm not sure who should make that decision, though. In short, though, this
        command works as intended and there is no bug here, per se.

        Show
        Justin Lee added a comment - create-http-listener does, in fact, do this to maintain funcationality of v2. create-network-listener does not as this is consistent with other creates in that it creates only the one element. I'm not opposed to doing this but if we go this route, we might as well have create-network-listener just be an alias for create-http-listener. And that's ok with me, too. I'm not sure who should make that decision, though. In short, though, this command works as intended and there is no bug here, per se.
        Hide
        sankarpn added a comment -

        I agree that the command is working as intended and there is no bug, but If I
        file it as a RFE its not going to be done for V3. This is a good time to
        implement it right and make it more developer friendly.

        cc'ing Jerome and Bill for their comments.

        Show
        sankarpn added a comment - I agree that the command is working as intended and there is no bug, but If I file it as a RFE its not going to be done for V3. This is a good time to implement it right and make it more developer friendly. cc'ing Jerome and Bill for their comments.
        Hide
        Justin Lee added a comment -
            • Issue 10193 has been marked as a duplicate of this issue. ***
        Show
        Justin Lee added a comment - Issue 10193 has been marked as a duplicate of this issue. ***
        Hide
        Justin Lee added a comment -

        this isn't a defect

        Show
        Justin Lee added a comment - this isn't a defect
        Hide
        Bill Shannon added a comment -

        I haven't spent a lot of time looking at this, but...

        If --transport or --threadpool aren't specified, and the create-network-listener
        command can create or use default transports or thread pools, I agree that that
        would be a good thing to do.

        But if --transport or --threadpool are specified, I would expect them to
        reference existing elements, which would need to be created separately.

        Show
        Bill Shannon added a comment - I haven't spent a lot of time looking at this, but... If --transport or --threadpool aren't specified, and the create-network-listener command can create or use default transports or thread pools, I agree that that would be a good thing to do. But if --transport or --threadpool are specified, I would expect them to reference existing elements, which would need to be created separately.
        Hide
        sankarpn added a comment -

        >If -transport or --threadpool aren't specified, and the create-network
        >listener command can create or use default transports or thread pools, I
        >agree that that would be a good thing to do.

        In this case referencing the defaults will be good, since the developer didn't
        specify threadpool/transport it might mean that he doesn't bother to tinker
        those entities and defaults are good enough for him.

        >But if --transport or --threadpool are specified, I would expect them to
        >reference existing elements, which would need to be created separately

        Good too, but if the elements doesn't exist and it is specified we should fail
        the command to let the user know that he should create those elements first.

        I just want to make sure that there is a trail for the developer to tell what
        needs to be done instead of letting him dig through the documents to make things
        work.

        Show
        sankarpn added a comment - >If - transport or --threadpool aren't specified, and the create-network >listener command can create or use default transports or thread pools, I >agree that that would be a good thing to do. In this case referencing the defaults will be good, since the developer didn't specify threadpool/transport it might mean that he doesn't bother to tinker those entities and defaults are good enough for him. >But if --transport or --threadpool are specified, I would expect them to >reference existing elements, which would need to be created separately Good too, but if the elements doesn't exist and it is specified we should fail the command to let the user know that he should create those elements first. I just want to make sure that there is a trail for the developer to tell what needs to be done instead of letting him dig through the documents to make things work.
        Hide
        Justin Lee added a comment -

        Using the deafult transport/threadpools would work. create-http-listener
        already tries to grab those and creates new ones if they can't be found. Now,
        this creates an implicit requirement that those names don't change (e.g., the
        default transport is named "tcp" and the default threadpool is
        "http-threadpool-1") but that's ok most of the time.

        Requiring that any explicitly named threadpools/transports already exists is the
        right way, imo. It reduces the number of unintended silent modifications that
        might not be what the user wanted. create-http-listener creates these
        structures on the fly but that's more to preserve compatibility with v2 behavior
        than anything else.

        If this is what we want then this one should get reassigned to nandini as this
        is her area. If you can confirm, bill, that this is what you'd like to see then
        I'll reassign this one.

        Show
        Justin Lee added a comment - Using the deafult transport/threadpools would work. create-http-listener already tries to grab those and creates new ones if they can't be found. Now, this creates an implicit requirement that those names don't change (e.g., the default transport is named "tcp" and the default threadpool is "http-threadpool-1") but that's ok most of the time. Requiring that any explicitly named threadpools/transports already exists is the right way, imo. It reduces the number of unintended silent modifications that might not be what the user wanted. create-http-listener creates these structures on the fly but that's more to preserve compatibility with v2 behavior than anything else. If this is what we want then this one should get reassigned to nandini as this is her area. If you can confirm, bill, that this is what you'd like to see then I'll reassign this one.
        Hide
        Bill Shannon added a comment -

        I'm not sure whether it's better to use the existing (default?) transport
        or thread pool, or whether it's better to create new ones just for use by
        this listener (as create-http-listener does, apparently), at least in the
        case where none was specified on the command. You'll have to assess which
        approach would be more useful and least surprising to the user.

        But if one is specified on the command, I think it's fair to say that it
        must already exist.

        Show
        Bill Shannon added a comment - I'm not sure whether it's better to use the existing (default?) transport or thread pool, or whether it's better to create new ones just for use by this listener (as create-http-listener does, apparently), at least in the case where none was specified on the command. You'll have to assess which approach would be more useful and least surprising to the user. But if one is specified on the command, I think it's fair to say that it must already exist.
        Hide
        Justin Lee added a comment -

        excluding from the v3 release schedule

        Show
        Justin Lee added a comment - excluding from the v3 release schedule
        Hide
        Tom Mueller added a comment -

        Updated summary to reflect what this RFE is about.

        Show
        Tom Mueller added a comment - Updated summary to reflect what this RFE is about.

          People

          • Assignee:
            Justin Lee
            Reporter:
            sankarpn
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated: