Re: [BuildStream] Responsive, but not overly verbose UX - not-in-scheduler

[Benjamin Schubert <contact benschubert me>]

Hey all,

I think having a single UI is definitely a good idea and would make things simpler. I agree with the high-level plan this thread is heading to.

I just have a single concern about it, the UI is already very slow (turns out it's even a bottleneck sometimes), so I would be very careful if we make it more complex that we don't make it slower than before or prevent some optimizations there. Also, we probably don't need the fully flegged UI when running in non-interactive mode, so we might want to have two different implementations for better performance.

Cheers,
Ben






-------- Original Message --------
On Apr 23, 2019, 11:17 AM, Tristan Van Berkom via buildstream-list < buildstream-list gnome org> wrote:

Hi William,

On Tue, 2019-04-23 at 10:23 +0100, William Salmon via buildstream-list
wrote:
[...]
> While this area of code is being re-factored I wonder if we could think 
> about if everything that `app.initialized` dose is needed for every 
> command? I suspect that it dose but if it dose not we may be able to 
> return some of the snappiness of the the simple `fast` commands that 
> have all be slowed by having to run a full `app.initialized`.
>
> I think the ideal would be that `app.initialized` was really fast and 
> that all the commands used it in full but as that dose not seem likely 
> in the short term it might be worth having a quick think about making 
> parts optional.

I think we definitely agree on this yes, some things have been tightly
coupled which might not have been a good idea.

E.g. see my comment here about the ArtifactCache being coupled with
Context not necessarily being a good thing:

https://gitlab.com/BuildStream/buildstream/issues/996#note_161287708

I think what we want from `App.initialized()` is:

* A Stream object to call APIs on from the CLI, calling these APIs
can result in initialization of subsystems that the frontend need
not understand, but the Stream is the main interface for the
frontend.

I would personally be fond of not having an App.stream member, but
rather have the CLI use a construct like this:

with app.initialized() as stream:
stream.build(...)

This would help to ensure it is *impossible* to call Stream APIs
without an initialized application (doesnt really make much of a
difference, but sends a clearer message to developers who open
up the cli.py file and implement new things).

* We want the Context initialized

This should be very lightweight, and basically the Context should
only have knowledge of the preferences, possibly overridden by
command line arguments.

* We want logging initialized, this essentially just setting some
pointers, having this as early as possible makes it possible to
have consistent logging experience for all commands.

* We want the main toplevel try/catch here to report BstErrors
consistently through a single non-zero exit status point

So yeah, I think the Context has become more loaded than necessary,
certainly the Platform need not be initialized for every invocation.

Also, another alternative is to allow the Context to be the owner of
subsystems like the artifact cache, but to always ensure that things
are always lazily resolved (i.e. initializing the Context doesnt mean
we know the artifact cache usage, until we actually *ask* for the
current usage).

Makes more or less sense ?

Cheers,
-Tristan

_______________________________________________
buildstream-list mailing list
buildstream-list gnome org
https://mail.gnome.org/mailman/listinfo/buildstream-list