Just reviewed the changes for the following and they are looking good:
- StartDatabaseCommand
- DatabaseCommand
- StopDatabaseCommand
I can also review changes for:
- CreateDomainCommand
- DeleteDomainCommand
- StartDomainCommand
- MonitorCommand
Thanks for these changes - much nicer to just use the @Param, instead of
having to use prepare() method.
Thanks,
Jennifer
On 3/21/2010 6:13 AM, Bill Shannon wrote:
> This is a followup to my message a few days ago.
>
> I've put a webrev of the changes so far at:
> http://javaweb.sfbay/~shannon/glassfish/webrev-asadmin/
> (Sorry, internal Oracle/Sun only. If you want me to send you a
> copy, contact me privately. It's about 2MB.)
>
> There are some infrastructure changes here that are pretty extensive,
> but the changes to commands are less complex. I'd love to have people
> review any of these changes.
>
> For example, I'd really like someone to make sure that I didn't make any
> stupid typos when I retyped the names of options, or didn't make a
> required option optional, or anything like that.
>
> Below you'll find an example of the changes to a typical command.
>
> One change you'll see in some commands was to undo a bad decision I made
> for 3.0. In 3.0 I allowed each command to decide whether to handle the
> asadmin "program options" (--host, --user, etc.). This resulted in some
> unintentional inconsistencies and some complicated code in some of
> the commands to handle (e.g.) --terse without handling all of the
> program options. This is all gone. Now all commands will accept
> all program options, even if the particular command has no need for
> them.
>
> If anyone has a chance to review any of this, please let me know.
>
> Thanks!
>
>
>
> Index: ListCommandsCommand.java
> ===================================================================
> --- ListCommandsCommand.java (revision 36047)
> +++ ListCommandsCommand.java (working copy)
> @@ -1,7 +1,7 @@
> /*
> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER.
> *
> - * Copyright 2008-2009 Sun Microsystems, Inc. All rights reserved.
> + * Copyright 2008-2010 Sun Microsystems, Inc. All rights reserved.
> *
> * The contents of this file are subject to the terms of either the GNU
> * General Public License Version 2 only ("GPL") or the Common Development
> @@ -42,6 +42,7 @@
> import java.util.regex.*;
> import org.jvnet.hk2.annotations.*;
> import org.jvnet.hk2.component.*;
> +import org.glassfish.api.Param;
> import com.sun.enterprise.admin.cli.util.CLIUtil;
> import com.sun.enterprise.universal.i18n.LocalStringsImpl;
> import static com.sun.enterprise.admin.cli.CLIConstants.EOL;
> @@ -61,41 +62,25 @@
> private String[] remoteCommands;
> private String[] localCommands;
> private List<Pattern> patterns = new ArrayList<Pattern>();
> +
> + @Param(name = "localonly", optional = true)
> private boolean localOnly;
> +
> + @Param(name = "remoteonly", optional = true)
> private boolean remoteOnly;
> +
> + @Param(name = "command-pattern", primary = true, optional = true,
> + multiple = true)
> + private List<String> cmds;
> +
> private static final String SPACES = "
> ";
>
> private static final LocalStringsImpl strings =
> new LocalStringsImpl(ListCommandsCommand.class);
>
> @Override
> - protected void prepare()
> - throws CommandException, CommandValidationException {
> - /*
> - * Don't fetch information from server.
> - * We need to work even if server is down.
> - * XXX - could "merge" options if server is up
> - */
> - Set<ValidOption> opts = new LinkedHashSet<ValidOption>();
> - addOption(opts, "localonly", '\0', "BOOLEAN", false, "false");
> - addOption(opts, "remoteonly", '\0', "BOOLEAN", false, "false");
> - addOption(opts, "help", '?', "BOOLEAN", false, "false");
> - commandOpts = Collections.unmodifiableSet(opts);
> - operandType = "STRING";
> - operandName = "command-pattern";
> - operandMin = 0;
> - operandMax = Integer.MAX_VALUE;
> -
> - processProgramOptions();
> - }
> -
> - @Override
> protected void validate()
> throws CommandException, CommandValidationException {
> - super.validate();
> - localOnly = getBooleanOption("localonly");
> - remoteOnly = getBooleanOption("remoteonly");
> -
> if (localOnly && remoteOnly) {
> throw new CommandException(strings.get("listCommands.notBoth"));
> }
> @@ -106,8 +91,9 @@
> throws CommandException, CommandValidationException {
>
> // convert the patterns to regular expressions
> - for (String pat : operands)
> - patterns.add(Pattern.compile(globToRegex(pat)));
> + if (cmds != null)
> + for (String pat : cmds)
> + patterns.add(Pattern.compile(globToRegex(pat)));
>
> /*
> * If we need the remote commands, get them first so that
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
> For additional commands, e-mail: admin-help_at_glassfish.dev.java.net
>
>
--
Thanks, Jennifer x81033 (440-709-5095)
Oracle <http://www.oracle.com>
Jennifer Chou
Phone: +1 44 20 7409 7350 <tel:+1%2044%2020%207409%207350> | Mobile: +1
44 755 283 8283 <tel:+1%2044%20755%20283%208283>
| London, England
Green Oracle <http://www.oracle.com/commitment> Oracle is committed to
developing practices and products that help protect the environment