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

Added --json output to conan export_pkg command #3809

Merged
merged 11 commits into from Oct 29, 2018

Conversation

Projects
None yet
4 participants
@danimtb
Copy link
Member

commented Oct 19, 2018

Changelog: Feature: Added --json output to conan export_pkg command

  • Refer to the issue that supports this Pull Request: closes #3654
  • If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
  • I've read the Contributing guide.
  • I've followed the PEP8 style guides for Python code.
  • I've opened another PR in the Conan docs repo to the develop branch, documenting this one.

@danimtb danimtb added this to the 1.9 milestone Oct 19, 2018

@ghost ghost assigned danimtb Oct 19, 2018

@ghost ghost added the stage: review label Oct 19, 2018

@lasote lasote requested review from memsharded and jgsogo Oct 20, 2018

@lasote lasote assigned memsharded and unassigned danimtb Oct 20, 2018

@danimtb danimtb changed the title Return action recorder result in conan api export_pkg Added --json output to conan export_pkg command Oct 22, 2018

@ghost ghost assigned danimtb Oct 22, 2018

@danimtb danimtb removed their assignment Oct 23, 2018

Show resolved Hide resolved conans/test/utils/tools.py Outdated
Show resolved Hide resolved conans/client/conan_api.py

@ghost ghost assigned danimtb Oct 23, 2018

@jgsogo

jgsogo approved these changes Oct 23, 2018

danimtb added some commits Oct 25, 2018

@danimtb

This comment has been minimized.

Copy link
Member Author

commented Oct 26, 2018

Included new fields in output to make the "exported" status more explicit:

{
    "error":false,
    "installed":[
        {
            "recipe":{
                "id":"pkg2/1.0@danimtb/testing",
                "downloaded":false,
                "cache":false,
                "exported":true,
                "error":null,
                "remote":null,
                "time":"2018-10-26T10:50:32.009158",
                "dependency":false,
                "name":"pkg2",
                "version":"1.0",
                "user":"danimtb",
                "channel":"testing"
            },
            "packages":[
                {
                    "id":"5825778de2dc9312952d865df314547576f129b3",
                    "downloaded":false,
                    "cache":false,
                    "exported":true,
                    "error":null,
                    "remote":null,
                    "time":"2018-10-26T10:50:32.082190",
                    "built":false
                }
            ]
        },
        {
            "recipe":{
                "id":"pkg1/1.0@danimtb/testing",
                "downloaded":false,
                "cache":true,
                "exported":false,
                "error":null,
                "remote":null,
                "time":"2018-10-26T10:50:32.069160",
                "dependency":true,
                "name":"pkg1",
                "version":"1.0",
                "user":"danimtb",
                "channel":"testing"
            },
            "packages":[

            ]
        }
    ]
}
@@ -22,7 +22,8 @@ def test_simple_fields(self):
self.assertFalse(my_json["error"])
self.assertEquals(my_json["installed"][0]["recipe"]["id"], "CC/1.0@private_user/channel")
self.assertFalse(my_json["installed"][0]["recipe"]["dependency"])
self.assertTrue(my_json["installed"][0]["recipe"]["cache"])
self.assertTrue(my_json["installed"][0]["recipe"]["exported"])

This comment has been minimized.

Copy link
@danimtb

danimtb Oct 26, 2018

Author Member

@lasote Unfortunately adding recipe as exported to recorder in conan create makes it to be marked as exported but not as cache. I think this is OK, but changes behavior

This comment has been minimized.

Copy link
@jgsogo

jgsogo Oct 26, 2018

Member

The ActionRecorder retrieves only the last action (see here). It's something I came accross when working in the cpp_info issue and I wonder if it makes sense to show all the actions related to a recipe in the json output (not to be done in this PR, but maybe it is worth considering it).

This comment has been minimized.

Copy link
@lasote

lasote Oct 26, 2018

Contributor

It was getting the latest action because until now it was the only needed, but it would be very very easy to pass all the actions to the get_doc_for_ref and use all to calculate the "export" field and maybe cache == not download or something similar, right?

This comment has been minimized.

Copy link
@lasote

lasote Oct 26, 2018

Contributor

By the way, maybe we have to remove the "cache" attribute, because it only means download=False

@memsharded memsharded assigned lasote and unassigned danimtb and memsharded Oct 26, 2018

Show resolved Hide resolved conans/client/conan_api.py Outdated

@ghost ghost assigned danimtb Oct 26, 2018

@lasote

lasote approved these changes Oct 26, 2018

@lasote

This comment has been minimized.

Copy link
Contributor

commented Oct 26, 2018

Merge when pass

error = None if the_action.type != INSTALL_ERROR else the_action.doc
def get_doc_for_ref(the_ref, the_actions):
errors = [action.doc for action in the_actions if action.type == INSTALL_ERROR]
error = None if not errors else errors[0]

This comment has been minimized.

Copy link
@jgsogo

jgsogo Oct 26, 2018

Member

Why not let the user know about every error:

errors = {action.name: action.doc for action in the_actions if action.type in [...error types...]}

and the output will show a dict with all the errors.

This comment has been minimized.

Copy link
@lasote

lasote Oct 29, 2018

Contributor

I think the error is and should be unique here, when an error is recorded it raises an exception.
Even though, there is no action.name and with a dict you will loose the order so it should be a list of dicts.
Anyway I will leave it as it is until someone requires something different, currently I think it won't make sense.

@memsharded memsharded merged commit 0e816e4 into conan-io:develop Oct 29, 2018

2 checks passed

continuous-integration/jenkins/pr-head This commit looks good
Details
license/cla Contributor License Agreement is signed.
Details

@ghost ghost removed the stage: review label Oct 29, 2018

grisumbras pushed a commit to grisumbras/conan that referenced this pull request Dec 27, 2018

Merge pull request conan-io#3809 from danimtb/feature/3654
Added --json output to conan export_pkg command
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.