[Kimchi-devel] [PATCH] [WoK] Asynchronous UI notification implementation

[Aline Manera]

+1

On 02/28/2017 11:12 AM, Daniel Henrique Barboza wrote:
> Just realized studying the code that, perhaps, the websockify token 
> considers
> only the file name, not the file path. This could mean that we can choose
> the dir of the unix socket freely, as long as the canonical name stays
> the same. So /run/woknotifications and /run/user/<u-id>/woknotifications
> would be accessed by the same URL. I'll investigate and see if it's true.
>
> Let me summarize here what I am planning for v2 reading all the replies:
>
> - Change the socket dir with as little backend changes as possible. 
> Assess if
> maintaining the same file name allows for the same token (URI) in the UI,
> avoiding using another API to deliver the token;
>
> - Change the push server initialization. Move it to server.py to avoid 
> execution
> in the unit tests, but allow execution in the test mode. Do the same 
> treatment
> with the ws_proxy initialization;
>
> - Simplify the UI code. I think there's a way to get rid of the 
> listener identifier
> without losing functionality;
>
> - Insert notifications in base model level. Put code in base WoK 
> classes to allow notifications
> for every model event without the need of adding it manually;
>
> - Standardization of messages sent. Send messages in the pattern 
> plugin/entity/action;
>
> - Minor code fixes (imports, remove kimchi references, etc).
>
>
> Let me know if I am missing something.
>
>
> Daniel
>
>
> On 02/28/2017 10:27 AM, Aline Manera wrote:
>>
>>
>> On 02/28/2017 10:02 AM, Lucio Correia wrote:
>>> On 27/02/2017 18:30, Daniel Henrique Barboza wrote:
>>>>
>>>>
>>>> On 02/27/2017 06:18 PM, Lucio Correia wrote:
>>>>> On 27/02/2017 18:12, Daniel Henrique Barboza wrote:
>>>>>>>>> +BASE_DIRECTORY = '/run'
>>>>>>>> Suggestion to use:
>>>>>>>> os.path.join('/run/user', str(os.getuid()))
>>>>>>>> in order tests may be run with root.
>>>>>>
>>>>>> I would prefer to choose a path that can be written by anyone 
>>>>>> else to
>>>>>> allow the push_server
>>>>>> to be started without root.
>>>>>
>>>>> Sorry, I meant *without* root.
>>>>> /run/user/<UID> allows for that, it's writable by the user that
>>>>> started wokd, be it root or not.
>>>>>
>>>> This change would require further UI changes to allow the /config API
>>>> (or other) to inform
>>>> the UI of the current websocket URL. This will not solve the problems
>>>> I've seen with the unit tests
>>>> though - multiple instances of the push_server will not be possible 
>>>> and
>>>> the unit tests will
>>>> break.
>>>
>>> I'm not seeing the relation between that BASE_DIR and the URL used 
>>> by UI. Anyway, I believe we both agree that just one server and one 
>>> URL is fine.
>>>
>>> My only concern is the directory to save that woknotifications file 
>>> and the permissions required for it.
>>>
>>>
>>
>> If the websockets directory path would be returned by a function in 
>> config.py (for example) the tests may overwrite that to set a /tmp 
>> dir and that way allowing it to run without root permissions.
>>
>> Daniel helped me to use @mock.patch and seems it can be a good way to 
>> go to solve it.
>>
>>>>
>>>> We need to discuss an alternative where:
>>>>
>>>> - any user can start the push_server, as you require
>>>
>>> I'm not requiring it. Just saying that if it will be started by 
>>> tests, it needs to run without sudo (i.e. use paths in filesystems 
>>> that allow for that).
>>>
>>> As I mentioned earlier, testing for 'test' option is not enough to 
>>> avoid PushServer to be started during tests, since as of now 'test' 
>>> option only governs which model will be used.
>>>
>>>
>>>>
>>>> - if we're to allow the push_server to be run in test_mode*, we 
>>>> need to
>>>> think in a way of
>>>> generating random paths that can be written by any user as well. 
>>>> You are
>>>> working closely
>>>> in Debian changes. What system dir can be used that can be written by
>>>> any user and,
>>>> preferably, exists in RPM distros too?
>>>
>>> On Fedora 25, /run/user/<UID>/libvirt is the default dir used to 
>>> store runtime data by libvirt when it is started with 
>>> "qemu:///session" url (regular user session).
>>>
>>> In fact it seems to be default place for that kind of usage in both 
>>> distros:
>>>
>>> Ubuntu 16.04:
>>> $ ls /run/user/1000/
>>> dbus-session  gnome-shell  gvfs-burn  pulse systemd 
>>> upstart-dbus-bridge.4451.pid upstart-udev-bridge.2946.pid
>>> dconf         gvfs         keyring    speech-dispatcher upstart 
>>> upstart-file-bridge.4451.pid upstart-udev-bridge.4451.pid
>>>
>>> Fedora 25:
>>> $ ls /run/user/1000/
>>> bus  libvirt  systemd
>>>
>>>
>>
>> _______________________________________________
>> Kimchi-devel mailing list
>> Kimchi-devel at ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>