Add lineage attribute across commands

[kyleknap]

For each of the command classes, a lineage attribute was added. The purpose of the lineage attribute is to be able to tell how to get to a specific command came from in the CLI. The lineage attribute is represented by a list of all of the commands that came before that specific command (in order) including the command itself. So for these commands, here are their lineages:

• aws ec2 --> ['ec2']
• aws ec2 run-instances --> ['ec2', run-instances']
• aws s3 ls --> ['s3', 'ls']
• aws s3api wait object-exists --> ['s3api', 'wait', 'object-exists']

This attribute helps unify the data-driven and custom commands because you can see how commands are linked together. Before, figuring out how to get to a command tended to be a little difficult and unreliable because the logic in figuring it out differed across command class and sometimes would require parsing event names.

I did my best to plumb this through to all of the custom commands, but if I missed one, let me know.

cc @jamesls @danielgtaylor

[coveralls]

Coverage increased (+0.02%) to 91.76% when pulling f800c7e on kyleknap:parent-commands into 7350250 on aws:develop.

[jamesls]

I'm concerned about mandating changes to every instantiation of command objects throughout our codebase. Seems like it's easy to miss one. I also think this clutters the __init__ interface by requiring lineage to be passed down.

Is there a reason we can't just do this dynamically and fill in this value during the building of the command table (without requiring them at instantiation time)? We know the parents during the building process.

Also, I think it would be more valuable to just have the parents be a list of the command objects themselves. You can always access the name property from the parent, and gives us a way to expand on this if we need to retrieve more info in the future.

[kyleknap]

@jamesls The hardest part was trying to pass in the parents for custom commands that hooked in via the event system (there was not really a way to do that at instantiation without passing it through __init__). I think what I am going to do instead is set the lineage after the event for creating command table is emitted. That way all commands in the command_table (even if they are hooked in) will get it added. What do you think?

I also like the idea that we just pass the command objects for its lineage. The only thing that we will have to work on is decreasing the lack of parity between the various command objects (which is a goal of ours). For example, the operation object does not have a name property right now.

[jamesls]

Yeah, sorry, after the event is fired was what I meant. If we just created a property on the command objects we set after we were done firing the command table event, then every command would automatically get this populated.

Also I'd be in favor of having a more consistent interface between the custom commands and the stuff in clidriver. Adding a name property would be great.

[coveralls]

Coverage decreased (-0.0%) to 91.74% when pulling 2126ce9 on kyleknap:parent-commands into 8781605 on aws:develop.

[coveralls]

Coverage decreased (-0.2%) to 91.55% when pulling 8d15a4a on kyleknap:parent-commands into 8781605 on aws:develop.

[kyleknap]

Sorry for all of the coveralls comments. These are real weird. The main reason it drops so much in the latest comment is because of s3 but I did not touch that module. Then sometimes it does not drop such as in the comment above it.

In the end there are a few lines that I do miss and that involves not hitting some of the getters and modifiers for the properties that I added in the cli driver. Do I need tests for those? I noticed that we do not test the attributes of the service and operation command anywhere else in clidriver.py.

@jamesls Let me know what you think. It is ready to get looked at again.

[jamesls]

@kyleknap I'd prefer to add tests. The fact that we didn't previously test the properties doesn't really seem like that good of a reason to not test the new properties. Seems like it would pretty simple tests anyways.

[coveralls]

Coverage decreased (-0.08%) to 91.67% when pulling 23c9f45 on kyleknap:parent-commands into 8781605 on aws:develop.

[coveralls]

Coverage increased (+0.08%) to 91.83% when pulling 23c9f45 on kyleknap:parent-commands into 8781605 on aws:develop.

[kyleknap]

@jamesls Cool. The tests seemed a little out of place at first because most of the tests in test_clidriver.py were more end-to-end testing, but I started a test suite for all of the different command classes in clidriver.py that we can build up.

Note that coveralls is still saying that my coverage dropped. However, all of the lines that I added are covered. I even covered some previous lines that were missing before in clidriver.py. It keeps saying the s3's tasks.py keeps dropping, even though I did not touch it.

Let me know what you think.

[jamesls]

Can't we do this without patching by just creating a new command class that has subcommands instead of using the BasicCommand instance? I'd prefer to avoid mocking these types of calls especially if they don't need to be.

[jamesls]

What about adding an integration test that works on a servicecommand/serviceoperation instance as well as a custom command? That way we can verify these values on actual real world commands?

Otherwise looks good, I much prefer this approach. Thanks for updating.

[jamesls]

For example (not sure what we'd need to do to get this into an integration test harness) but something along these lines:

In [19]: import awscli.clidriver

In [20]: driver = awscli.clidriver.create_clidriver()

In [21]: top_help = driver.create_help_command()

In [22]: [cmd.name for cmd in top_help.command_table['cloudformation'].lineage]
Out[22]: ['cloudformation']  # < --- verify this is cloudformation

# Then do a subcommand
In [24]: subcommand = top_help.command_table['cloudformation'].create_help_command().command_table['create-stack']

In [25]: [cmd.name for cmd in subcommand.lineage]
Out[25]: ['cloudformation', u'create-stack']   # < --- verify this is cloudformation, create-stack

# Maybe one for waiters as well.

[coveralls]

Coverage increased (+0.09%) to 91.84% when pulling b5356ee on kyleknap:parent-commands into 8781605 on aws:develop.

[coveralls]

Coverage increased (+0.23%) to 91.98% when pulling b5356ee on kyleknap:parent-commands into 8781605 on aws:develop.

[kyleknap]

@jamesls I incorporated your feedback and added integration test in my latest commit.

[jamesls]

Looks great, thanks for updating.