╭─[jdob]─[hinterlands]─[~]─[]─● ╰─➤ pulp-admin repo group members list --group-id g1 ╭─[jdob]─[hinterlands]─[~]─[]─● ╰─➤ We're not great about being consistent in these situations, but we should display a message showing there are no members in the group rather than a blank output.
I'm refactoring these to the client package itself and will fix as part of that.
Since you've taken this, I'll go ahead and make my case for why I think it shouldn't be done. When requesting a list of items, I think the only data that should be written to stdout is those items. I know that in general, pulp-admin's output is not particularly friendly for machine consumption, but that could easily change (I suspect) by perhaps adding an option to suppress the nice formatting of documents. If it will ever be the case that the output of pulp-admin will be used programatically (which I think we should aim to support), it will be a problem for a "no items found" type message to be written to stdout. The other option for displaying such a message is to write it to stderr, but that should of course be reserved for actual errors. I am assuming that the absence of members in a repo group should not be considered an error. The POSIX philosophy focuses heavily on the idea that command line tools should define clear formats for input and output so that tools can be used in combination with pipes. Examples of commands showing empty result sets: $ mkdir delme $ ls delme/ $ $ find . -name 'idontexist' $ $ grep idontexist /etc/hosts $ $ git diff # assuming there are no local changes $ $ lspcmcia # assuming you don't have any PCMCIA devices $ $ touch foo $ cat foo $ All of these commands are asked to list some output that may or may not exist, and none of them show a "no results found" type of message. In general, I think it's a good idea for pulp-admin to behave similarly to how other sysadmin tools behave, so that it will be intuitive and comfortable for the sysadmins who use it. Showing a "no results found" type of message would, in my opinion, contribute to pulp-admin being a systems tool that does not feel like a natural part of a linux system. Thoughts on that?
It's a really interesting argument. My gut reaction is that I think comparing pulp-admin and a more low level tool like find/grep/ls is apples and oranges, but I'm not 100% behind that statement. Part of me feels that we're more of a user application than a system tool, but again that's subjective and vague. And more importantly, I'm not an admin, so I may be in the minority there. I'm with you on the suppression option. I've done this once before in the CLI (I forget where) with a --quiet (aliased to -q) flag that produced programmatic output. That was more of a proof of concept than anything; if we're going to go that route I think it should be across the board. For instance, I think all of our lists should support a --quiet option that uses a CSV-like syntax. So repo list: $ pulp-admin repo list +----------------------------------------------------------------------+ Repositories +----------------------------------------------------------------------+ Id: pulp-rhel6 Display Name: pulp-rhel6 Description: None Content Unit Count: 35 Notes: Would have: $ pulp-admin repo list -q pulp-rhel6,pulp-rhel6,,35, Things like delete will have to be addressed too. In moving from v1 to v2, I was adament about making sure we properly used the exit code to reflect failed commands. This is the next step: providing an interface that is purely driven off exit codes and not on user-readable output. While we're at that, I'd like to support it at the .conf file level. If a "default_quiet: True" config is set (not the real name, just a conceptual idea) the quiet option will be assumed in all cases. It could basically invert the CLI; a --verbose, -v option would be available instead to explicitly request the more fancy UI. So I'm definitely with you. However, the question is when/how we get there. I'd like to do the following: - "Fix" this bug anyway. I'm a fan of consistency, even if it's consistently not ideal. - Add a sprint story to address the entire CLI as a whole for this concept. That way we don't have a few sections doing one thing, others doing another, and potentially still others doing a third option. I'd rather keep them the same, put some thought into a more global policy for how we want to handle this, and then apply it across the board. I think it'll be much easier to change one pattern rather than multiple patterns into a new one. Let me know what you think.
commit 907a8065fd62bd9ac08330cdf8f559d2ef240a05 Author: Jay Dobies <jason.dobies> Date: Thu Aug 23 13:13:49 2012 -0400 - Refactored repo and repo group commands to the client package builtins/extensions/admin/pulp_repo/pulp_cli.py builtins/test/unit/test_pulp_repo_extension.py builtins/test/unit/test_repo_group_member_section.py builtins/test/unit/test_repo_group_section.py platform/src/pulp/client/commands/criteria.py platform/src/pulp/client/commands/options.py platform/src/pulp/client/commands/repo/cudl.py platform/src/pulp/client/commands/repo/group.py platform/src/pulp/client/extensions/core.py platform/src/pulp/client/extensions/extensions.py platform/test/unit/test_commands_repo_cudl.py platform/test/unit/test_commands_repo_group.py platform/test/unit/test_search_command.py
(In reply to comment #3) This sounds reasonable to me. I think it is worthwhile for our management tool to strive for interface continuity with the general POSIX environment, even though it is a different scale of tool than low-level commands, as you pointed out. Also, I think there are plenty of examples of more comprehensive user applications that also conform to the basic POSIX principles. For example, going back to the "ip" command which is responsible for managing a tremendous amount of complex networking setup, you can do things like this: $ ip tunnel # show a list of tunnels, of which there are none $ yum and rpm don't seem to behave the way I suggest, but I don't think they've ever won any awards for spectacular usability. Apt does behave this way: $ apt-cache search idontexist $ In any case, I vote for staying consistent with other tools in our environment even if pulp is a broader scale of application, and for writing only the requested type of data to stdout (Example: if the request is for a list of repos, write only repos to stdout). But I think it's totally reasonable to stay consistent internally for now and address this globally at a future date.
build: 0.327
[root@pulp-master ~]# rpm -q pulp-rpm-server pulp-rpm-server-0.0.328-1.fc17.noarch [root@pulp-master ~]# pulp-admin repo group create --group-id gp1 Repository Group [gp1] successfully created [root@pulp-master ~]# pulp-admin repo group members list --group-id gp1 +----------------------------------------------------------------------+ Repository Group Members +----------------------------------------------------------------------+ No matching repositories found
Pulp v2.0 released