glassfish
  1. glassfish
  2. GLASSFISH-13961

create-local-instance: lbenabled should be a boolean

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.1
    • Fix Version/s: 3.1_b28
    • Component/s: load_balancer
    • Labels:
      None
    • Environment:

      Operating System: All
      Platform: All

    • Issuezilla Id:
      13,961

      Description

      These are comments from the ASArch review of asadmin commands that are new to
      3.1 on 10/13/2010.

      The lbenabled option takes boolean values, but this isn't indicated in the usage
      message. The reason for this is that it is declared as a String:

      @Param(name="lbenabled", optional = true, acceptableValues = "true,false")
      private String lbEnabled;

      It should be declared as a boolean with an appropriate default value.

        Activity

        Hide
        kshitiz_saxena added a comment -

        This implementation exists for two commands : create-local-instance and
        create-instance. Setting lbenabled to Boolean will start reflecting a default
        value of false, which may not be the case. Default value is conditional and
        cannot be correctly reflected.

        Show
        kshitiz_saxena added a comment - This implementation exists for two commands : create-local-instance and create-instance. Setting lbenabled to Boolean will start reflecting a default value of false, which may not be the case. Default value is conditional and cannot be correctly reflected.
        Hide
        Tom Mueller added a comment -

        By "conditional", do you mean that lbenabled is not allowed for a stand-alone
        instance, but that for an instance that is part of a cluster, lbenabled by default
        is true? Thus, lbenabled really needs to have 3 values (null, true or false), and
        only null is allowed for a standalone instance.

        Show
        Tom Mueller added a comment - By "conditional", do you mean that lbenabled is not allowed for a stand-alone instance, but that for an instance that is part of a cluster, lbenabled by default is true? Thus, lbenabled really needs to have 3 values (null, true or false), and only null is allowed for a standalone instance.
        Hide
        kshitiz_saxena added a comment -

        There are multiple conditions
        1. Standalone instance : Null value. This option is not allowed at all.
        2. Clustered instance :
        a) If all existing instances in cluster are disabled, then default value is false
        b) If above condition does not apply, then use default value specified by system
        property org.glassfish.lb-enabled-default
        c) If system property is also not specified, then use default value of "true"

        Show
        kshitiz_saxena added a comment - There are multiple conditions 1. Standalone instance : Null value. This option is not allowed at all. 2. Clustered instance : a) If all existing instances in cluster are disabled, then default value is false b) If above condition does not apply, then use default value specified by system property org.glassfish.lb-enabled-default c) If system property is also not specified, then use default value of "true"
        Hide
        Bill Shannon added a comment -

        Sigh. Yes, it looks like this is a case for which there's no good way to
        indicate the default value automatically. You might consider providing a
        custom usage message that says:

        [--lbenabled[=<lbenabled>]]

        And then use Boolean in the implementation so that the CLI will allow the
        full boolean option syntax and will check that the value is a proper boolean
        value, while still allowing you to support the null case where the option
        isn't specified.

        Show
        Bill Shannon added a comment - Sigh. Yes, it looks like this is a case for which there's no good way to indicate the default value automatically. You might consider providing a custom usage message that says: [--lbenabled [=<lbenabled>] ] And then use Boolean in the implementation so that the CLI will allow the full boolean option syntax and will check that the value is a proper boolean value, while still allowing you to support the null case where the option isn't specified.
        Hide
        kshitiz_saxena added a comment -

        Fix targeted for ms07.

        Show
        kshitiz_saxena added a comment - Fix targeted for ms07.
        Hide
        kshitiz_saxena added a comment -

        Changed String to Boolean and added usage text.

        Show
        kshitiz_saxena added a comment - Changed String to Boolean and added usage text.
        Hide
        kshitiz_saxena added a comment -

        Changed String to Boolean and added usage text.

        Show
        kshitiz_saxena added a comment - Changed String to Boolean and added usage text.

          People

          • Assignee:
            kshitiz_saxena
            Reporter:
            Tom Mueller
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: