omni: make more library like

ahelsing commented 9 years ago

We claim omni is a library. But really we run the whole end to end library using omni.call when we want to do anything, passing all the command line arguments as an array, including the string command. Awkward.

It would be better if

you could instantiate a handler, so all the startup stuff is done only once. Then invoke the methods
you could pass the arguments to the command directly, without having to produce an array of strings
you could call the methods in the AM API (and soon, federation API) directly with the types that the API calls for

Imported from trac ticket #388, created by ahelsing on 10-03-2013 at 08:30, last modified: 02-14-2014 at 10:23 CCing: nick.bastin@...

ahelsing commented 9 years ago

nick.bastin attached 388-4.patch on 12-18-2013 at 14:32

ahelsing commented 9 years ago

nick.bastin attached 388-3.patch on 12-18-2013 at 14:10

ahelsing commented 9 years ago

nick.bastin attached 388-2.patch on 12-18-2013 at 14:06

ahelsing commented 9 years ago

nick.bastin attached 388.patch on 12-18-2013 at 13:56

ahelsing commented 9 years ago

Today, you could create an AMCallHAndler with a framework, config, and opts. Or a CHCallHAndler. Or if you don't know which or need both, a CallHandler.

You would need to do:

    framework, config, args, opts = initialize(argv, options)

And then:

        handler = CallHandler(framework, config, opts)

Then each time you want to invoke a method, you would do:

        result = handler._handle(args)

But all that still requires that you have an argv to start with. That argv and options input to initialize is used in a call to parse_args, and then it configures logging, loads AM nicknames, loads the omni config file, and then creates the framework.

Also note that the AMCallHandler caches the config, opts, and the list of clients to talk to. So to do a 2nd command with different options/AMs might require resetting those appropriately. Additionally, AMCallHandler calls _correctAPIVersion - which you might want to skip or do only once in some contexts.

Other points: the args list to methods in AMCallHandler gets mostly processed by the utility method _args_to_slicecred, but not entirely. EG createsliver looks at args[1] for the rspec file. If all this were done in a utility that would help. Even better of course would be if the real work of these methods were done in a separate function that someone else could call if they wanted.

Trac comment by ahelsing on 10-03-2013 at 08:39

seledwards commented 9 years ago

Nick reports omni is hard to import because of the need to set the PYTHONPATH.

Trac comment by sedwards (github user: seledwards) on 12-17-2013 at 09:41

ahelsing commented 9 years ago

The usage pattern inside another package is pretty odd, as omni.py is both a command line tool and the "library" entry point. What I really want to do is move all of the non-script files (and directories) in gcf/src/ into a single package (gcf?), and then make an omni.py that is the command line app that calls into that package. Then any library user can just copy the larger package into their software, leaving the command line tools behind.

Thoughts?

Trac comment by nick.bastin on 12-17-2013 at 10:47

ahelsing commented 9 years ago

Splitting the command line tool and library entry point seems a reasonable idea. And at the same time, making it easier to construct arguments, re-use a handler and framework and logger between omni calls, etc.

I'm not quite clear on what the directory structure would look like when done in your proposal, so perhaps you can sketch it out for me?

In the end, I just want stuff that works now to continue working, and for installation of the existing command line tools to be no more than unzipping in place. Changes that respect that basic point seem fine I think, but I'd like to see them.

Trac comment by ahelsing on 12-17-2013 at 10:52

ahelsing commented 9 years ago

src/
      omni.py
      gcf/
            omnilib/
            geni/
            sfa/
            __init__.py
            oscript.py

Trac comment by nick.bastin on 12-17-2013 at 11:06

ahelsing commented 9 years ago

What is oscript.py? All the other scripts? If so, I think that buries things like clear-passphrases and readyToLogin too much.

I'd pick something other than 'gcf'. But I'd need to understand what you have in mind with all the other things that were in src to come up with a name. The scripts that start the GCF AM and CH and whatnot could go in a 'gcf' parallel to 'geni' if you want. But lets use a different name for files that relate to Omni.

Trac comment by ahelsing on 12-17-2013 at 11:52

ahelsing commented 9 years ago

oscript.py is what omni.py used to be. The new omni.py exists in the same place and just imports oscript as omni.

Trac comment by nick.bastin on 12-17-2013 at 12:22

ahelsing commented 9 years ago

And all the other scripts also stay where they used to be?

Can we just move oscript inside omnilib/, and not create the extra directory? And maybe rename it as omniscript?

Trac comment by ahelsing on 12-17-2013 at 19:48

ahelsing commented 9 years ago

BTW, this isn't really this ticket, but since it came up: there are a number of uses, such as monitoring, for which installing omni/gcf by unpacking a tarball is subideal, and having (at least) a debian package would be a lot better. I don't want to distract y'all from the main purpose of this ticket, which sounds good, but IMO if we wind up moving directories and whatnot, i think we should have an eye towards future ease-of-packaging.

Trac comment by chaos on 12-18-2013 at 09:33

ahelsing commented 9 years ago

Initial patch attached that moves things around so they can be safely embedded inside larger packages. There's likely still a bunch of cleanup in the scripts that originally imported omni.py directly.

Trac comment by nick.bastin on 12-18-2013 at 13:56

ahelsing commented 9 years ago

Another patch added that cleans up the rest of the python scripts inside the src dir.

Trac comment by nick.bastin on 12-18-2013 at 14:07

ahelsing commented 9 years ago

And yet another patch that cleans up the scripts in the examples directory.

Trac comment by nick.bastin on 12-18-2013 at 14:10

ahelsing commented 9 years ago

Final patch cleans up stitcher.

Trac comment by nick.bastin on 12-18-2013 at 14:32

ahelsing commented 9 years ago

This seems to be working now, is on develop, and will be in the next release.

Thanks, Nick!

Trac comment by ahelsing on 02-14-2014 at 10:23

GENI-NSF / geni-tools

omni: make more library like #388