Skip to content
This repository was archived by the owner on Oct 10, 2020. It is now read-only.

Enable dbus install#887

Closed
baude wants to merge 1 commit intoprojectatomic:masterfrom
baude:dbus_install
Closed

Enable dbus install#887
baude wants to merge 1 commit intoprojectatomic:masterfrom
baude:dbus_install

Conversation

@baude
Copy link
Member

@baude baude commented Feb 13, 2017

Enable for performing atomic install over dbus.

@baude
Copy link
Member Author

baude commented Feb 13, 2017


@polkit.enable_proxy
def Install(self, image, name=None, user=False, system=False, remote="", setvalues=None, extra_args=None):
def Install(self, image, name=None, system=False, remote=False, storage=None, user=False, setvalues= None, extra_args=None):
Copy link
Member

Choose a reason for hiding this comment

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

Looks like you have a bogus space after setvalues.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

atomic_dbus.py Outdated
def Install(self, image, name=None, system=False, remote=False, storage=None, user=False, setvalues=None, extra_args=None):
if setvalues is None:
setvalues = []
if extra_args is None:
Copy link
Member

Choose a reason for hiding this comment

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

Should we do the extra check for whether setvalues and exta_args if passed in is a list? Actually I think dbus would fail if they were not a list.

extra_args is weird, since it needs to precisely be setup by the user to match the internals of the Atomic python library.
I take it, this is just for debuging?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not for debug. My understanding is that extra_args needs to be a list. So if nothing is passed (i.e. None), we just pass an empty list.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added assertions that set_values and extra_args must be a list.

Copy link
Member

Choose a reason for hiding this comment

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

Do you ever see someone passing extra_args across dbus? I think we should just remove it from the dbus inteface and hard code it to [ ]

Copy link
Member Author

Choose a reason for hiding this comment

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

I have no idea. I was just following suit. I'll remove it and hard set it like you said.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

Copy link
Member

Choose a reason for hiding this comment

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

I meant hard code it on the server side and don't allow it to be passed from the client side.

Copy link
Member Author

Choose a reason for hiding this comment

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

Rebased and fixed.

@baude baude force-pushed the dbus_install branch 4 times, most recently from dc4f631 to 0425569 Compare February 16, 2017 15:39
@baude
Copy link
Member Author

baude commented Feb 17, 2017

@rhatdan should be ready

Enable for performing atomic install over dbus.
@baude
Copy link
Member Author

baude commented Feb 17, 2017

@rh-atomic-bot r+ 7a02e29

@rh-atomic-bot
Copy link

⌛ Testing commit 7a02e29 with merge 6dcca7b...

@rh-atomic-bot
Copy link

☀️ Test successful - status-redhatci
Approved by: baude
Pushing 6dcca7b to master...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants