Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[discussion] Api on the fly #3005

Closed

Conversation

oliver-sanders
Copy link
Member

closes the CLI component of #2123

This is built atop #3004 (look at last commit).

This is a quick (<2hr) demonstration to show just how easy it is to bring api-on-the-fly to the Cylc CLI in the new network framework.

Api-on-the-fly in a nutshell (for those not in the meeting when this was discussed (2 years back?)):

  • Request the suite runtime API from the suite itself.
  • Version independence between clients.
    • No need to change cylc versions (export CYLC_VERSION=...) to work with suites at different Cylc version on the CLI.
    • No need to hard-code GUI forms (e.g. control=>stop_suite or control=insert_tasks) let the suite provide the interface.
  • No more duplication of documentation, all APIs derive from the docstrings defined at the endpoints.
  • No more inconsistency between GUI, CLI and network functionality, add a feature in the network and its available everywhere.
  • No more type errors, strings getting interpreted as lists or other nonsense.
  • Restricted to user-orientated commands, internal commands like cylc message would suffer performance wise.

Examples

Note we would probably symlink commands (e.g. cylc stop <suite> to cylc client <suite> stop). An endpoint rationalisation would be needed too.

$ # get list of endpoints supported by the suite
$ cylc suite suite api
["api", "clear_broadcast", "describe", "dry_run_tasks", "expire_broadcast", ...]
$ # get the CLI help for the set_stop_cleanly endpoint
$ cylc suite generic set_stop_cleanly --help
usage: set_stop_cleanly [-h] [--kill_active_tasks KILL_ACTIVE_TASKS]

Set suite to stop cleanly or after kill active tasks.

returns: tuple - (outcome, message)
    
    outcome (bool)
        True if command successfully queued.
    message (str)
        Information about outcome.
    

optional arguments:
  -h, --help            show this help message and exit
  --kill_active_tasks KILL_ACTIVE_TASKS
                        If True the suite will attempt to kill any active
                        (running, submitted) tasks
$ # call the set_stop_cleanly endpoint
$ cylc suite generic set_stop_cleanly       
[true, "Command queued"]

Loose ends

  • Validation / client-side casting
  • Generate HTML forms for the GUI?
  • Niceness (see TODOs in code)

Questions

Will GraphQL replace everything or do we want to keep some (simplified/tidied) endpoints for Cylc "transactions" (e.g. stop)? Or should this be re-developed on-top of the GraphQL endpoint?

@oliver-sanders oliver-sanders self-assigned this Mar 14, 2019
@matthewrmshin matthewrmshin added this to the cylc-8.0.0 milestone Mar 14, 2019
@oliver-sanders oliver-sanders changed the title Api on the fly [dicsuccion] Api on the fly Mar 15, 2019
@sadielbartholomew sadielbartholomew changed the title [dicsuccion] Api on the fly [discussion] Api on the fly Mar 16, 2019
@hjoliver
Copy link
Member

hjoliver commented Mar 18, 2019

@oliver-sanders - forgot to comment the other day sorry. This looks good and useful to me. I'm not sure how GraphQL impacts it, let's discuss in the meeting...

@dwsutherland
Copy link
Member

dwsutherland commented Mar 18, 2019

@oliver-sanders - Even though the owner could interact directly with their UI Server, via graphiql interface (which is self-documenting), I think it will focus on web gui provision right? You could could garner the graphiql docs info via a CLI also I suppose, but I guess we would need to provide a command to send queries strings (to avoid people having to use curl)..

Nice work BTW.

@hjoliver
Copy link
Member

Presumably there's a way of getting the same info that Graphiql does, for the CLI, without using curl?

@dwsutherland
Copy link
Member

Presumably there's a way of getting the same info that Graphiql does, for the CLI, without using curl?

Yeah urllib3 (supports TCP/web-sockets?).. The information is available via a query (introspection). So with this and graphql we have both workflow service and ui server api-on-the-fly.

@oliver-sanders
Copy link
Member Author

Questions:

  1. Should we push ahead with this or will it become superseded by graphql?
  2. Should we auto-generate the CLI and or GUI forms via this approach?
  3. Are we happy with cylc suite stop <name> or would we rather have cylc stop invoke cylc suite?
  4. Are we happy with the docstring parsing approach (assuming we cache results or pre-compute on startup), it makes Sphinx a Cylc dependency?

all("." not in arg for arg in args[1:])):
if (len(args) in [2, 3]
and all("/" not in arg for arg in args)
and all("." not in arg for arg in args[1:])):
items = [(args[0] + "." + args[1])]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Task globs do work for inserting task proxies.

@hjoliver
Copy link
Member

(One for the meeting agenda...)

@dwsutherland
Copy link
Member

Questions:

  1. Should we push ahead with this or will it become superseded by graphql?

I think this will hold alot of value for the CLI, whereas the UI Server is decoupled from suite versions differently, and GraphiQL interface uses the schema.py object/field descriptions and docstrings differently. It uses them as markdown for then interface, i.e.:
mutation_taskActions

  1. Should we auto-generate the CLI and or GUI forms via this approach?

CLI definitely, probably too prescriptive for graphql mutations? separate GUI forms

  1. Are we happy with cylc suite stop <name> or would we rather have cylc stop invoke cylc suite?

We are deprecating "suite" for "workflow".. Unless we plan on having a separate CLI for the uiserver, then cylc stop over cylc flow stop? (I guess we would need to revive a urllib3 client if we were going to channel through the UI Server... I would think CLI for workflow interaction would only talk to workflows, thoughts?)

  1. Are we happy with the docstring parsing approach (assuming we cache results or pre-compute on startup), it makes Sphinx a Cylc dependency?

How do we ensure CLI consistency with workflow CLI independence? (not a new problem I suppose)

@oliver-sanders
Copy link
Member Author

A quick and obvious note on CLI efficiency, the cylc suite command implemented here could easily check the API version from the contact file and only request the API spec if the CLI API version differs from the workflow API version. This would make it possible to use this approach for commands like cylc message without introducing a performance overhead.

@oliver-sanders
Copy link
Member Author

Should we push ahead with this or will it become superseded by graphql?

Main reason I brought this up is at Melbourne we had been talking about implementing a graphql endpoint alongside the REST endpoint. I couldn't see any reason to maintain two interfaces instead of one and graphql is much much better at self-documentation etc.

The remaining headache from the API on the fly front of keeping both interfaces is that the cylc suite command (or its renamed successor) would have to be able to work with both (to allow direct suite comms or comms via the UI Server) and produce identical CLI.

@oliver-sanders oliver-sanders mentioned this pull request Jul 2, 2019
3 tasks
@oliver-sanders
Copy link
Member Author

I think we have now reached the consensus that the GraphQL API will be used for the CLI so I'll mark this as POC and bump it back to #2123.

@matthewrmshin matthewrmshin added POC Proof of Concept superseded labels Jul 2, 2019
@matthewrmshin matthewrmshin removed this from the cylc-8.0.0 milestone Jul 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
POC Proof of Concept superseded
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants