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

Add in a flag to make commands output machine-readable results instead of human readable ones #566

Open
vladfrangu opened this issue May 13, 2024 · 7 comments
Assignees
Labels
adhoc Ad-hoc unplanned task added during the sprint. enhancement New feature or request. t-tooling Issues with this label are in the ownership of the tooling team.

Comments

@vladfrangu
Copy link
Member

Context: this issue ties in perfectly with #497 (specifically #497 (comment))

Currently, almost all our commands (excluding actor get-input, get-value, push-data and set-value) return strictly human readable content.

For example, when running apify call apify/hello-world, what you'll get is something like this:

Code - 2024-05-13 at 08 14 58@2x

This is fine for normal users:

  • it sometimes prints our update checker (which needs to be moved to stderr)
  • it prints the fact the actor is getting called
  • it prints the logs of the run
  • it returns the actor run detail shortcut
  • and then ends with a success/error message, whichever is the status

This is not processable at ALL with CLI tools. A lot of CLI tools have a way to return just a JSON blob that can passed down to jq, curl, or w/e users want.

We should add in a flag to all commands that print user-readable data but could also print machine-readable data. I propose either --api, --raw or any other names you can come up with. Regardless of the name, it must be a flag that can be expected on all commands, is not likely to clash with other params, and is clear enough.

We also need to decide whether certain logs should be printed to stderr instead of stdout (like the logs for an actor call). We also need to decide on adding a flag for that to not print the logs, but that can be sort-of achieved with setting wait-for-finish to 0 and not printing logs in that case (or should, thats another bug :) )

CC @jancurn @netmilk @B4nan @barjin

@vladfrangu vladfrangu added enhancement New feature or request. adhoc Ad-hoc unplanned task added during the sprint. labels May 13, 2024
@vladfrangu vladfrangu added this to the 1.0 milestone May 13, 2024
@vladfrangu vladfrangu self-assigned this May 13, 2024
@jancurn
Copy link
Member

jancurn commented May 13, 2024

+1 to this. In Actor specification we suggested to use --json to force JSON input/output for all commands.

@vladfrangu
Copy link
Member Author

In Actor specification we suggested to use --json to force JSON input/output for all commands.

I'd be fine with this too, although I'm worried it could clash with future options maybe? Is this something we should make a poll about on slack, or just go with what the spec says?

@jancurn
Copy link
Member

jancurn commented May 13, 2024

No polling is Slack please. We need to prepare a comprehensive and consistent specification of the new CLI, agree on the interface, and then decide in which order we'll add the features. But the design must be done at once, to avoid breaking changes or surprises down the line. I asked @netmilk to take charge of the CLI design, we need to kick this work off soon (cc @mtrunkat)

@B4nan
Copy link
Member

B4nan commented May 13, 2024

There is still one very important unresolved question, do we want to ship v1 now, or wait for this new API? Because there are lots of internal improvements we did over the past quarter and we planned to ship it already, only to see @barjin presenting the new API proposal in the sprint we wanted to ship :]

I think it would make sense to ship it now, because the new proposal might take weeks if not months to be agreed on, and it would be a shame to hold the improvements we have in the master branch for too long. Maybe it would make sense to ship it as v0.20 if we plan to do API overhaul soon, since the changes in master are mainly internal (plus a few features and smaller BCs). Or we can just wait, doing both in v1 would be IMO the best, I am just afraid that the discussion could be lengthy (implementation should be rather simple).


Worth noting that we currently build v1 beta releases already from master, so if we decide to ship it as v0.20 it might be confusing for those who already tried the beta. Hard to guess if there are people outside of Apify who tried it.

@janbuchar
Copy link
Contributor

There is still one very important unresolved question, do we want to ship v1 now, or wait for this new API?

I believe the --raw-output flag (or whatever we settle upon) is orthogonal to the new API and it can be added whenever, with or after v1, as it should be backwards compatible. Am I missing something?

Maybe it would make sense to ship it as v0.20 if we plan to do API overhaul soon

Agreed, it'd be weird to release v1 while already planning to overhaul the API and release v2.

We also need to decide whether certain logs should be printed to stderr instead of stdout (like the logs for an actor call).

This is the way. It should also be a different issue though.

@netmilk
Copy link
Contributor

netmilk commented May 13, 2024

My personal perspective here:

There's nothing necesiraly wrong about releasing 1.x.x with the minor breaking changes and then, even shortly after that, releasing 2.x.x, introducing the new design of the CLI API.

I'd love to see the release (deployment) conventions streamlined across all our tooling, focused on merging every meaningful work as soon as possible to the master and release instead of hoarding work in branches. The sooner users use it, the sooner we have feedback, and the sooner we can move on.

I'd love to disconnect the versioning from emotions as semantic-relese intends, and the communication around major improvements wrap around named milestones like The new CLI API design instead of v1.

How does that sound?

@B4nan
Copy link
Member

B4nan commented May 13, 2024

I think it makes more sense to ship 0.20 now, since we haven't really done a major API overhaul since the very beginning, and now we are likely gonna end up with that - I hope we can all agree that such thing suites more into a v1 release (in the end you were the one pointing out v1 should be about stabilizing the API, what we did until now did not touch the user-facing API very much).

I also don't like to hoard stuff in branches (and we don't do that here, the work is in the master branch for quite some time, available for testing via beta releases), on the other hand, breaking releases are the one case where I feel it's a good thing - good for our users to be more specific. Who would want to see multiple breaking releases over a short period, having to think about upgrading guides and whatnot?

@vladfrangu vladfrangu removed this from the 0.20 milestone May 13, 2024
@fnesveda fnesveda added the t-tooling Issues with this label are in the ownership of the tooling team. label May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
adhoc Ad-hoc unplanned task added during the sprint. enhancement New feature or request. t-tooling Issues with this label are in the ownership of the tooling team.
Projects
None yet
Development

No branches or pull requests

6 participants