qri-io / qri

you're invited to a data party!
https://qri.io
GNU General Public License v3.0
1.1k stars 66 forks source link

Automation on the CLI planning #1788

Open ramfox opened 3 years ago

ramfox commented 3 years ago

What can a user do on the CLI with regards to automation? Are we expected to have feature parity with the frontend? The frontend is able to glosses over the difference between "save with transform" and "run workflow" by only allowing users to interact with transforms in the context of a workflow. There are many questions that didn't have to be answered because of this set up & I'm wondering if we should actually be taking more cues from the UI when it comes to designing the CLI commands.

We currently have qri save --apply to run a transform, qri apply to dry run, and will soon have some kind of qri workflow command that we can also use to trigger a manual run of transform, via the workflow.

Having commands that do very similar things with very different effects may come to bite us in the butt. Specifically the relationship between commands like:qri save --apply & qri workflow --run-now.

Technically both are "manually triggered" but one is more "automated", could have the same dataset output, but have different effects on the whole system (one keeps a record of the run & increments the run count, one doesn't). We may eventually have a repeat of the qri export/qri get rigmarole, where users are confused about how they are supposed to be using qri.

We have a solid understanding/hypothesis for how workflow will be used in the UI, since we have a design plan that has been vetted by potential users. We don't have the same understanding for the CLI. Can we use the UI to inform how a user will expect to use transforms and workflows?

Matching closer to the UI this would mean: qri save is for "manual" saves (user supplied changes) only & we remove the ability for a user to add a transform this way (to the actual command, not necessarily to the underlying lib function) qri apply for dry running qri workflow is for adding and updating triggers, transforms, and completors, & manually triggering a run of the entire workflow (and eventually can be used to debug any completors): qri workflow save --file would run and save a new transform qri workflow run manually triggers the workflow, w/ flags for not including completors ... further qri workflow commands for updating the triggers & completors other commands for viewing run output

Implications:

Specifically thinking about the relationship between the UI and CLI. If a user in UI land clicked "run now" & then didn't see a record of the run in the workflow's activity log, it would be confusing. I think it would be similarly confusing for a CLI users. Having multiple options to run a transform, via qri save(or qri apply for a dry run) & qri workflow seems dangerous.

Wanted to start this discussion now, in case it informs the work on the automation systems.

Arqu commented 3 years ago

I really don't have any real answers or high quality input, so I'm just speaking on it from a "casual user perspective".

Honestly apply feels like an inside thing that you need to know about while the rest is fairly obvious on first read. In our API spec discussions when we were grouping them etc, I did comment once to just make it transform which comes back in the from qri transform ds similar to qri save ds while apply is more like qri apply >>something<< ds.

Anyways, this is not to steer this into a naming debate, we had these already, my point is that I think you're laying out the right approach to focus on having some user feedback (or the next best thing we have) on this similar to the product/front end side of things.

ramfox commented 3 years ago

To your point about user feedback @Arqu, I'm struggling with how to get this. I think maybe looping @chriswhong into this discussion might be useful.

I figured presenting some potential separation of concerns & get feedback from the team on proposed commands would be a good first round.

dustmop commented 3 years ago

I'm not really familiar with the proposed qri workflow command, as I've been pretty off in the weeds working on other things. My vague understanding is that it's wrapping save and transforms, and then also adds triggers and completion hooks which are the core of qrimatic. Is that accurate? If so, I totally agree that it seems like a footgun to keep around the old qri save --apply if that ends up losing important information (RunID and the like).

The qri save --apply command was added as a way to clear up the semantics of what a transform is, how it relates to the save path, and especially how they both interact with FSI, as motivated by the apply rfc. If qri workflow is replacing those old use cases, there's not really a reason to keep the old way around.

chriswhong commented 3 years ago

I may not be grasping all of this properly, but I am in support of save being for manual creation of new versions of datasets, which could include saving a new transform but would never actually run the transform.

Running the transform is then the business of workflow, which will have commands for adding and removing triggers and completion tasks, but also have a command for manually executing the workflow.

I still see the need for dry running a transform to see what changes it will create if it actually runs, and this is neither workflow nor save... maybe just call it dryrun?

ramfox commented 3 years ago

My vague understanding is that it's wrapping save and transforms, and then also adds triggers and completion hooks which are the core of qrimatic. Is that accurate?

tbh we haven't talked about this via the cli yet, but on qrimatic/frontend, yes this is how it works. The user doesn't have a concept of saving or applying a transform without being in the context of a workflow, so qrimatic wrapped save and apply & we referenced qrimatic to "run now" or "dry run"

I'm basically wondering if we should be extending that to cli, since that flow is still in our UI design & it would end up simplifying some questions that keep cropping up that @b5 and I have discussed in previous automation design meetings.

Some lingering questions I had were if a user runs qri save --apply should that "run" be included in a run count? Should the output be kept in the run store? We decided, no because they are technically manual runs & the run store tracks things associated with workflows. But then I did a bit of a thought experiment. If I was a UI user and ran "run now", I would expect that run to end up in my run activity. If I'm a CLI user & I run qri save --apply, see all the same transform output, see that there is a new version & then look at my run log and not see that same transform output, I would probably be confused. Same with run count.

Coding wise, this change would be as simple as removing --apply from qri save, and allowing the orchestrator.RunWorkflow method to take an optional transform, that it passes down to lib.Save.

ramfox commented 3 years ago

which could include saving a new transform but would never actually run the transform.

whoa I never even thought of that as a possibility

chriswhong commented 3 years ago

whoa I never even thought of that as a possibility

I think in the UI when I deploy, there's a commit which saves the transform and saves a bunch of workflow settings. But if I "deploy and run", it does all of the above AND runs the transform, which could make a new version. So I could have two new versions if I do "deploy and run", and I think it makes sense to separate the commit where the transform is updated from the commit where the data is updated by the new transform...

dustmop commented 3 years ago

which could include saving a new transform but would never actually run the transform.

This is already possible (on CLI) by running qri save --no-apply --file transform.star me/my_dataset

I still see the need for dry running a transform to see what changes it will create if it actually runs, and this is neither workflow nor save... maybe just call it dryrun?

The intention of qri apply is that it can also be used with FSI to modify the body file in a working directory, without saving a new commit. However, that never got implemented. I know FSI has really been on the back burner for a while, but unless we get rid of it and abandon this plan, calling it "dryrun" doesn't fit this use case.

Another question (related to FSI being on the back burner), is how workflows interact with working directories.

ramfox commented 3 years ago

Another question (related to FSI being on the back burner), is how workflows interact with working directories.

Do you mean specifically allowing users to edit triggers and completors via the filesystem?

If you mean, what happens when a "checked out" dataset gets run via automation, I think it should interact however running qri save --apply currently works... I think. If a working directory is "dirty" does qri save --apply run, or is there an error?

ramfox commented 3 years ago

I think it makes sense to separate the commit where the transform is updated from the commit where the data is updated by the new transform

Okay thank you for bringing this up, I forgot to be considering this while designing the system: the expectation that adding the transform is not the same commit as applying the transform.

This is already possible (on CLI) by running qri save --no-apply --file transform.star me/my_dataset

ahh yes, ty. Forgot this existed.

dustmop commented 3 years ago

I think it should interact however running qri save --apply currently works.

The current behavior is kinda of broken, and we started recommending FSI and transforms not be used at the same time.

The outlined plan in the RFC was that qri apply / qri save --apply would modify the body (under the assumption that you want the file present locally on your filesystem so that you could simultaneously view it in another program), and that body could be inspected after it had been changed by the transform but before it was saved. Manual edits would be detected (by techniques not deeply discussed in the RFC) and rejected if they conflict with the result of the transformation. This seemed to match expected behavior of those users who were banging on FSI at the time, but might be reasonable to abandon, depending on if we want workflows to work with FSI at all.

If we do, probably worth having the workflow's user editable data (triggers and completion hooks) be present as you mentioned, yes.

ramfox commented 3 years ago

ahhhh. Okay, will need to think about it more. Feels like that needs its own deep dive of discussion.

b5 commented 3 years ago

nice to see folks bringing their respective expertise to the table. Generally I think a nice consensus is emerging, and there's less to be afraid of than it seems. I would call out agreement with Dustin's comment:

I totally agree that it seems like a footgun to keep around the old qri save --apply if that ends up losing important information (RunID and the like).

We should drop drop --apply flag from save, and it could/should coincide with the introduction of a new workflow command. qri save --no-apply should become the only possible behavior when working with the save command (that is, any changes to the transform component are NOT executed when running qri save).

To me the big question left on the table is what qri apply executes: transforms, workflows, or both. We have two lanes here:

  1. qri apply executes transform components, and we describe it as a niche plumbing command that mainly powers dry run.
  2. qri apply executes workflows, and we add options to apply like a --dry-run flag that change execution behavior.

The first presents weirdness for the concept of a run, because now we want runs to be records of executing a workflow. The upside of executing transforms is a user can comfortably know that apply won't do "trigger stuff" like send email.

On the other hand, if apply executed workflows, we're very close to what @ramfox is describing with qri workflow run. If we made it possible to supply just a transform component / script file, we might be able to pack all of the functionality of qri apply into qri workflow run, and drop the apply command. Examples:

If all of that sounds right, then we're headed for a world where you don't run transforms, only workflows can be run. the old qri apply is now just a workflow with no triggers or hooks specified.

ramfox commented 3 years ago

Glad we've come to consensus around qri save: remove the --apply flag from CLI & make the default behavior no-apply when adding a transform.

The main questions surrounding apply vs workflow run --dry I think boil down to: what would be the expected difference in dry running the transform vs dry running a workflow. The only difference I can see is the expectation around hooks, specifically: what does it mean to "dry run" a hook? I assume it would be different for each Completor (the system that runs the hook). Eg, would a PushHook connect with the specified remote & ensure it has access to push? Would an EmailHook send a test email?

If this is behavior we (eventually) want, then yes a workflow dry-run is definitely different than a transform dry run, and it makes sense that we need separate commands to do it. Then, I fully buy qri workflow run --dry me/dataset, even if, right now, it does the same actions as qri apply me/dataset, since we don't have any hooks implemented yet.

Is this the expected behavior of the following proposed commands: qri workflow run --file transform.star

qri workflow run --file transform.star me/dataset