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

[Cli 2.0]: Pipe conan commands POC #7696

Closed
wants to merge 57 commits into from

Conversation

czoido
Copy link
Contributor

@czoido czoido commented Sep 10, 2020

This PR is a proof of concept about the idea of being able to pipe conan commands. It has minimal implementations of the commands create and upload to demonstrate how after the creation of a package the upload process can take advantage of the results in stdout of the create command.
Do export the environment variable: export CONAN_V2_CLI=1 before trying.
Possible uses:

$ conan create path/conanfile.py --user=carlos --channel=experimental | conan upload
pkg/1.0@carlos/experimental#cfeb566fb51ca21a2f549c969c907b53:587de5488b43bc9cebd0703c6c0f8c74#cfeb566fb51ca21a2f549c969c907b53
$ conan create path/conanfile.py --user=carlos --channel=experimental | conan upload --output json
{
    "uploaded_references": [
        "pkg/1.0@carlos/experimental#cfeb566fb51ca21a2f549c969c907b53:587de5488b43bc9cebd0703c6c0f8c74#cfeb566fb51ca21a2f549c969c907b53"
    ]
}

You can also take the json output and use something like jq to parse the json output from a previous conan command:

$ conan create path/conanfile.py --user=carlos --channel=experimental --output json | jq ".full_reference" -r | conan upload --output json
{
    "uploaded_references": [
        "pkg/1.0@carlos/experimental#cfeb566fb51ca21a2f549c969c907b53:587de5488b43bc9cebd0703c6c0f8c74#cfeb566fb51ca21a2f549c969c907b53"
    ]
}

The piping capability is added in the conan_command or conan_subcommand decorators. This can be discussed if the user must have total control over this, maybe we want to take it outside there and define in the commands.
The positional argument it can substitute then has to be marked as optional so that Argparse accepts the input argument when passed explicitly or the piped stdin when detected a piped command.

conans/cli/cli.py Outdated Show resolved Hide resolved
conans/cli/command.py Outdated Show resolved Hide resolved
conans/cli/commands/create.py Outdated Show resolved Hide resolved
conans/cli/commands/create.py Outdated Show resolved Hide resolved
nargs="?")
parser.add_argument("-r", "--remote", action=OnceArgument,
help="Upload to this specific remote")
parser.add_argument("--query", default=None, action=OnceArgument,
Copy link
Member

Choose a reason for hiding this comment

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

This would be a perfect candidate to get out of here, and pipe the result of conan search --query into conan upload

conans/client/api/conan_api.py Outdated Show resolved Hide resolved
@czoido czoido marked this pull request as draft September 15, 2020 13:31
@czoido czoido marked this pull request as ready for review October 28, 2020 15:45
@czoido czoido added this to the 1.32 milestone Oct 29, 2020
Copy link
Member

@memsharded memsharded left a comment

Choose a reason for hiding this comment

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

I am not convinced about this approach. It is basically one abstraction pipeable instead of directly defining a pipe argument to argparse, that in any case needs to be parsed by each command, there is no automatic mapping to the corresponding argument.

Besides that small architecture detail, I don't feel this provides the most UX friendly flow. While it seems correct conceptually and it matches the linux command line, that is for "simple" commands piping, like ls, cat, grep, and similar low level system utilities. But conan create does a ton of things, and conan upload does another tons of thing. I feel that both in CI and user usage, it might be better to do one thing at a time, and then use file outputs or other mechanism to let the information flow. Plus having an intermediate file that can be provided to conan upload for example, would enable further automation by users, that could create that file themselves.

I think we want to delay this decision for later, and mature first the real cases we want to implement, like create + upload (what if create has --build=missing and build more things? should return something more than the reference being created?), and if there is a common pattern, we will factor it. What do you think @czoido ?

@czoido
Copy link
Contributor Author

czoido commented Nov 16, 2020

Yes, I agree. Maybe we want something more powerful than just piping commands. For me, as I saw this, piping between commands would only work for some commands and some cases but and maybe it's not really solving any problem or improving flows. Maybe it would, but I think it's a good idea to study the real world cases we want to improve deeper and then see how different approaches work would be the best.

@memsharded memsharded modified the milestones: 1.32, 2.0 Nov 16, 2020
@solvingj
Copy link
Contributor

solvingj commented Mar 3, 2021

Here are some thoughts surrounding the work and discussion done so far. TLDR; the suggestion is to resume work on adding pipe support for Conan 2.0, with some more clear parameters and goals, and more feedback from users.

  • I believe it was an unlucky and unfortuate choice to start with conan create and conan upload. It definitely looks like a really good place to start because it's such a common workflow, with a known problem, but clearly led to awkward scenarios. Then, based on this one example, the whole concept was deemed flawed, which doesn't seem fair to general case.

  • This work was effectively unseen by most of the community and users from the time it started to the time it stopped (as evidenced by the complete lack of comments and feedback). None of the historical github issues and/or comments suggesting that pipe support would be helpful were linked to this PR. There is no sign that we mentioned or requested additional input from any of those users on github or slack. It seems likely that some users would be able to review the history here and add insight, clarity, or more practical use cases beyond conan create | conan upload. We should open a fresh discussion and seek that feedback from those users if we move forward.

  • I agree with the negative comparison of conan create to ls, cat, grep. These are two very different categories of commands. Specifically, conan create is MUCH higher level command, defining an entire workflow: a sequence of different operations which all get logged to a single output stream. Trying to implement pipeable option to make the output suitable for pipeling seems logical, but I think it will never be a good fit for this reason.

  • Continuing from the point above, we can imagine that the conan create process could have been implemented in terms of smaller primitive commands which needed to be composed together to achieve the same result. It's clear that the conan create command was designed with simplicity and ease-of-use as the top priority: a one-stop-shop command that does "it all" when creating a package. It's great for most users. The alternative seems like it would obviously be more work for the user, more opportunity for error, and less attractive for the general case, and much less desirable.

  • However, contrary to the previous "obvious" conclusion, we do in-fact have a growing number of users pursuing that very alternative, with very good reasons, and we are in-fact accommodating those cases with a complete re-design of the "local flow". The new flow is based on the more primitive commands, creating more flexibility between steps. We're also considering really cool ideas like "project caches" for intermediate builds, and other things which take us further from conan create as being the cornerstone of package creation.

  • With that in mind, we can probably now say more confidently that it will probably be more valuable to explore support pipe on other commands which are not conan create. We can evaluate the usage of pipe in the primitive commands "new local flow", for example conan export.

  • Finally, and perhaps even most interestingly, I think the "predicated value" of adding "pipe support to some Conan Commands" in a general sense might be much higher if we open the a design and brainstorming session with thinking of brand new primitive conan commands which do either new or existing conan operations in a way that is pipe/unix-oriented first. Obviously, this is a bit of a rabbithole, so the suggestion isn't to just come up with (and develop and maintain) a bunch of redundant un-necessary conan commands. Instead, I think it's the kind of brainstorming exercise which can yield important insights and opportunities for big improvements which we might be missing right now in a number of areas.

Here's an example of my own actual suggestion from 2019:

 conan makeref <path_to_conanfile> <user>/<channel>

I believe there are many more opportunities for new and useful bridge commands like this.

@memsharded
Copy link
Member

Conan 2.0 new CLI is almost there, and so far we haven't found solid and clear cases of Conan command piping, so it seems this might not be necessary atm.

@memsharded memsharded closed this Feb 15, 2022
@czoido czoido removed this from the 2.0 milestone Feb 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
CLI
Awaiting triage
Development

Successfully merging this pull request may close these issues.

None yet

3 participants