jedelman8 / cpal

The Common Programmable Abstraction Layer
22 stars 10 forks source link

Initial Command-line parsing #9

Closed yandy-r closed 10 years ago

yandy-r commented 10 years ago

--> Had typo in branch name on original request


Added command-line parsing capabilities, this allows cpal to be called in “script” like form. More methods and additions to be made, but this should be a good start to give an idea.

added beginCalls method to device(), making it a more modular and allowing for easy expansion later, init now just initializes the class variables.


few examples below:

normal call

$ python main.py
{   'connect_ip': '192.168.31.22',
    'free_system_memory': 48884,
    'hostname': u'leaf-2a',
    'platform': u'vEOS',
    'serial_number': '12345',
    'system_uptime': u' 09:13:24',
    'total_sytem_memory': 1000444,
    'var_name': 'r1',
    'vendor': 'arista'}

calling with IP address set (-i or --ipAddress) and manufacturer (-m or --manufacturer)

$ python main.py -i 192.168.31.21 -m arista
{   'connect_ip': '192.168.31.21',
    'free_system_memory': 190728,
    'hostname': u'leaf-1a',
    'platform': u'vEOS',
    'serial_number': '12345',
    'system_uptime': u' 09:13:33',
    'total_sytem_memory': 1000444,
    'var_name': 'r1',
    'vendor': 'arista'}

I can't run the onePK since no access to library, but here's an example changing the manufacturer to -m cisco, it crashes, but shows that tries to load the proper libraries.

$ python main.py -i 192.168.31.21 -m cisco
Traceback (most recent call last):
  File "main.py", line 193, in <module>
    main()
  File "main.py", line 164, in main
    r1 = device("r1", "arista", "192.168.31.22")
  File "main.py", line 90, in __init__
    self.beginCalls(obj)
  File "main.py", line 95, in beginCalls
    self._thisdevice = cisco(self.address, obj)
NameError: global name 'cisco' is not defined

try it out and let me know, this was all done under a different branch (comand-parsing) so may need to be merged with *master.

@jedelman8

jedelman8 commented 10 years ago

Typically, the arg parsing, etc. will happen in its own function, helping modularize it even more. Is this something you can do? It should not be in init either.

The arg parsing stuff looks good, but why aren't you using it when: if name == 'main'...to test it out? You just call main() and display facts. main() should call the arg parsing function and return whichever function or variable is requested.

Can you comment more on why you include 'obj' as a super class for the device class? I think what we were talking about was super classes for the vendor modules, not sure it's needed for device.

I'm also not sure how you made changes to the code, but it's very hard to decipher what's changed in your commits b/c there are large sections of green and red code. It's like you deleted what was there and just copy and pasted new. This makes it difficult to figure out what's going on. For others and for myself, it only highlights exactly the lines that changed.

When you want to commit to a core module like main.py, can you do it in smaller phases? It will be easier to review and merge back into master. I know it may be a pain, but can you do this for the work you've done, at least starting with the arg parsing?

yandy-r commented 10 years ago

the reason it shows so many lines, has to due with the tabs and spaces. My editor is set to convert tabs to spaces, once it did, thinks everything is a change.

object not obj was include because if you use python PEP8, it gives warnings/errors as to old style classes. It is typically good practice, starting I think from 2.3 to make all classes inherit from "object" which is built into python.

The changes were not that drastic, but it just looks like it. I can try and re-fork it I guess and change the editor, but would be counter productive.

I did add the parsing into if name == main, then moved it over to the class once it was tested. They are optional and not required, it does not affect normal initialization by passing arguments into the constructor.

I added a few comment blocks to show the actual changes made. The code really wasn't changed all the much, just annoying that it looks that way. It's all in main.py

as far as the main(), I'm jsut used to it and forgot to change it back, it's been changed in a separate commit.

The arg parsing stuff looks good, but why aren't you using it when: if name == 'main'...to test it out? You just call main() and display facts. main() should call the arg parsing function and return whichever function or variable is requested.

the -f part has not been implemented, right now you can pass -i (ip_address) and -m (manufacturer) and -n (name) for the var_name. Less changes to commit this way, and wanted to get an overview first.

If you make argument parsing a function outside of the class or even init, then that's more variables to pass along later. If it's built into the class, then it can be optional, and no need to change the arguments that are passed between method calls. The only other way is to make it a method of the class and call it within init.

python main.py -i 192.168.31.21 -m arista -n 'SW1'
{   'connect_ip': '192.168.31.21',
    'free_system_memory': 58340,
    'hostname': u'leaf-1a',
    'platform': u'vEOS',
    'serial_number': '12345',
    'system_uptime': u' 15:43:25',
    'total_sytem_memory': 1000444,
    'var_name': 'SW1',
    'vendor': 'arista'}