Merge lp://qastaging/~edgar-b-dsouza/acire/snippet_dependency_res_1 into lp://qastaging/acire

Proposed by Ed S
Status: Needs review
Proposed branch: lp://qastaging/~edgar-b-dsouza/acire/snippet_dependency_res_1
Merge into: lp://qastaging/acire
Diff against target: 518 lines (+480/-0)
5 files modified
acire/SnippetdependenciesDialog.py (+167/-0)
acire/utils.py (+138/-0)
bin/acire (+15/-0)
data/ui/SnippetdependenciesDialog.ui (+151/-0)
data/ui/snippetdependencies_dialog.xml (+9/-0)
To merge this branch: bzr merge lp://qastaging/~edgar-b-dsouza/acire/snippet_dependency_res_1
Reviewer Review Type Date Requested Status
Ed S (community) Needs Information
Jono Bacon Needs Fixing
Review via email: mp+21924@code.qastaging.launchpad.net

Description of the change

First-cut implementation of "snippet module dependency resolution", using only on-the-system software repositories.

Still pending ideas of how to query Launchpad for PPAs that supply packages used by snippets, that are not in official repos.

Had to switch code last-minute from using dpkg-query to using 'aptitude search' since dpkg-query was shortchanging me on the results :-( wonder if that's a bug!

To post a comment you must log in.
Revision history for this message
Jono Bacon (jonobacon) wrote :

Thanks for doing this work, Ed! I merged it into my local copy and I see you added some dialogs but I don't see how they are exposed in the UI, so I could not test it. How do I see it working?

Also, I think we need to think carefully about an elegant way of exposing this in the UI. My thinking of what could be cool would be to put a yellow bar at the top of the source view that says:

  This snippet requires additional Python modules to run. Click here to install them...

You would then click the yellow area and dialog box would inform you of what needs installing and grab them from the repos.

As for supporting third-party PPAs for modules, I think this is out of scope for a first cut: I would not bother focusing on that - just support the main repos.

One final consideration is that I would like to ensure this is as pluggable as possible for different distros: I don't want to make Acire depend on Ubuntu. So it could be useful to detect which distro Acire is running on and then have a plugin system that implements how Acire interacts with different distros. This means that if someone runs on Fedora as an example, the module search will use yum.

Thanks for your hard work, Ed!

review: Needs Fixing
Revision history for this message
Ed S (edgar-b-dsouza) wrote :

Hi, Jono.

The checking is hooked into bin/acire in the snippet_selected() function, just below your new docs_box code - search for:
"check_results = utils.check_script"

See it working:
a) Module with installation candidate package:
E.g. python-clutter (if installed on your system, you could temporarily uninstall it to test?). When you visit a Clutter snippet, SnippetdependenciesDialog should pop up and prompt you to install it.

b) Module with no installation candidate package:
In my case, that was Zeitgeist. If you have a PPA for Zeitgeist, and can temp-uninstall the package, temp-remove the PPA, and restart Acire to refresh the list of available packages - and then visit a Zeitgeist snippet. The Info Page button should take you to https://wiki.ubuntu.com/PythonSnippets_Dependencies (which I put up yesterday, please read/edit it?).

elegant way of exposing this in the UI:
I like your idea. I'd thought of notifying in statusbar, but that was too unobtrusive. Your yellow bar is hard for the user to miss. I'll look for a GTK control to use, and will propose a subsequent feature branch merge.

third-party PPAs for modules out of scope for a first cut:
Yes, I agree :-) that's why I called it a first cut :-). Like I asked in question #104601, (for a later feature branch), I need some pointers/ideas on how to go about that. OTOH, if you think the Wiki page I mentioned is a viable stopgap for some time, that's cool too.

as pluggable as possible for different distros:
Yes, I'd thought of that but wanted to first focus on feature addition; I find it a little complex to simultaneously add in such abstractions. If you like, I can work on that earlier (let me know) to:
- move the distro-specific functions into separate module scripts
- add distro-sniff code in the generic 'utils.py'
- use the distro-sniff to import the relevant pkg-handling module script

Happy to put in the work :-) and am hoping I may feature in authors/Help > About at some point...

Thanks,
Ed.

Revision history for this message
Jono Bacon (jonobacon) wrote :

Thanks Ed for the continued work on this: this is shaping up really nicely!

First, in terms of the UI, why don't you first add a GTK label at the top of the source view which says which modules are required and a button that allows someone to kick off the installation process.

In terms of supporting multiple distros, I love that you have kept the logic separated out into utils.py and it could be useful to first get it running for Ubuntu, we can then focus in on supporting other distributions and merge that logic into utils.py too.

I think for PPA element, the wiki page is ideal: to be quite frank, I am unlikely to accept a snippet if it is not installable from the archive anyway. :-)

Does this sound OK?

Thanks Ed, you rock the house. :-)

Revision history for this message
Ed S (edgar-b-dsouza) wrote :

Jono: sounds good, but I have a couple of questions.

Add GTK label & button - OK, will do that:

Q1. ...but show the Snippetdependencies dialog on click of the button, right? Or do you want all the relevant code from that .py file pulled in to clutter up bin/acire? :-)

Q2. Add them right away and push updated code to the same feature branch? Is that possible without messing up the review process? Or push it to another feature branch and propose that for merge instead? Sorry, still quite a n00b :-)

Re PPA - well, if that's dropped, I'm relieved :-P since it was quite a hurdle anyway.

Revision history for this message
Jono Bacon (jonobacon) wrote :

Hi Ed!

Q1 - my recommendation would be the following use case:

  1. User loads a snippet that does not have a module installed.
  2. Above the source view a little bar appears (like the plugin missing bar in Firefox) where it says what module(s) are missing and a button called 'Install'.
  3. User clicks the 'Install' button, a dialog box pops up to confirm what will be installed, the user clicks the 'OK' button, is asked for password and then a dialog displays progress of the installation.
  4. The bar disappears from the snippet.

I would prefer that the code for this feature does not clutter up bin/acire if possible. Probably best is that the logic lives in a file called moduleinstaller.py and there is a separate .py file with the dialog box.

Sound doable?

Q2 - I would like you to sync your branch to the current lp:acire branch and work in your worn branch. We can then test it until it is working and then I will merge it into lp:acire. :-)

Glad the PPA side of things is dropped: let's keep the core feature our focus.

Thanks, Ed, you rock! :-)

Revision history for this message
Ed S (edgar-b-dsouza) wrote :

On Wed, Mar 24, 2010 at 12:37 PM, Jono Bacon <email address hidden> wrote:
>  1. User loads a snippet that does not have a module installed.
>  2. Above the source view a little bar appears (like the plugin missing bar in Firefox) where it says what module(s) are missing and a button called 'Install'.
>  3. User clicks  the 'Install' button, a dialog box pops up to confirm what will be installed, the user clicks the 'OK' button, is asked for password and then a dialog displays progress of the installation.
>  4. The bar disappears from the snippet.
>
> I would prefer that the code for this feature does not clutter up bin/acire if possible. Probably best is that the logic lives in a file called moduleinstaller.py and there is a separate .py file with the dialog box.
>
> Sound doable?

Yes, but it will take me some time to do the changes, especially
replacing the gnome-terminal with progress dialog - need to research
for how to do that. Give me a few days.

> Q2 - I would like you to sync your branch to the current lp:acire branch and work in your worn branch. We can then test it until it is working and then I will merge it into lp:acire. :-)

I'll get the stuff above working first as you have asked, then update
my local repo to tip and meld in my changes, then push to a feature
branch on LP.

You didn't answer - overwrite/update this same feature branch which
I've proposed for merge, or push to a new one?

Thanks
Ed.

Revision history for this message
Jono Bacon (jonobacon) wrote :

Hi Ed!

No worries about it taking time: today I am going to try and get 0.5 released (which is the code currently in lp:acire). I don't plan on any changes to Acire in the next few weeks. This will provide a stable target for you to hack against.

As for branches: I don't mind if you use the same branch or a new one, so long as when the work is complete I have a branch that I can cleanly merge into lp:acire.

Ed, you rock, seriously! Also, we have 149 snippets, and Acire becomes more and more useful every day! :-)

Revision history for this message
Ed S (edgar-b-dsouza) wrote :

On Thu, Mar 25, 2010 at 11:58 PM, Jono Bacon <email address hidden> wrote:
> Hi Ed!
>
> No worries about it taking time: today I am going to try and get 0.5 released (which is the code currently in lp:acire). I don't plan on any changes to Acire in the next few weeks. This will provide a stable target for you to hack against.

Oh, good! Keeping on pulling revisions and merging code again is a bit
of a pain... yeah, I know that's how collaborative dev is done, but
still :)
Also, I could propose for merge a later feature branch (after you
agree we've got this dependency thing in and looking good) with some
quick changes I have in mind (below).

> As for branches: I don't mind if you use the same branch or a new one, so long as when the work is complete I have a branch that I can cleanly merge into lp:acire.

OK, thanks for clearing that up.

Yes, I understand... two-way merges are enough trouble for me, when it
gets more complicated... ugh! I've done my best to have each new
feature branch as a revision on tip; for Acire, where we don't have
many people working on it, this is easy to do, but for bigger teams...
project leader really has a tough time, I guess!

> Ed, you rock, seriously! Also, we have 149 snippets, and Acire becomes more and more useful every day! :-)

Heh - thanks! :-) If I apply for Ubuntu Membership, I'm going to ask
you to sign my page :-)

I have some quick ideas I'd like to try:
- add a pushbutton to clear the VTEterm. Also add a checkbox
"Auto-clear on each run". For the occasional snippet that does
terminal output, it can get confusing for the user to determine which
is the latest output, especially if user is modifying code in the
GTKSourceView.
- shift the Exec/Copy/SaveAs buttonbar below the SourceView - it's a
more logical place for it in the left-to-right, top-to-bottom flow for
the UI: load code in the sourceview using the combo and treeview,
(optionally) edit the code, hit Exec, see output in the VTETerm below.
- see if I can (at runtime) add the shortcuts for the three buttons
into their captions. Additionally, I'm thinking of changing the Copy
button shortcut... when I want to copy only a portion of the snippet
from the sourceview, I hit Ctrl-C and the button copies ALL the
snippet to the clipboard. I've been bitten several times (and I added
the shortcut! :-( ) -- I think other users might be annoyed too. Wish
we had more feedback.

Revision history for this message
Jono Bacon (jonobacon) wrote :

> Oh, good! Keeping on pulling revisions and merging code again is a bit
> of a pain... yeah, I know that's how collaborative dev is done, but
> still :)

:-)

> Also, I could propose for merge a later feature branch (after you
> agree we've got this dependency thing in and looking good) with some
> quick changes I have in mind (below).

Sounds good. :-)

Just so you know, I just released 0.4 (http://aciresnippets.wordpress.com/2010/03/26/acire-0-4-released/) - I may merge a few small changes in there and bug fixes: a few people have a few things that are working on.
>
> Yes, I understand... two-way merges are enough trouble for me, when it
> gets more complicated... ugh! I've done my best to have each new
> feature branch as a revision on tip; for Acire, where we don't have
> many people working on it, this is easy to do, but for bigger teams...
> project leader really has a tough time, I guess!

Indeed! While there will be a few changes here and there, I recommend you regularly update your branch to ensure you don't need to rebase it much. :-)

> > Ed, you rock, seriously! Also, we have 149 snippets, and Acire becomes more
> and more useful every day! :-)
>
> Heh - thanks! :-) If I apply for Ubuntu Membership, I'm going to ask
> you to sign my page :-)

Bring it on!

> I have some quick ideas I'd like to try:
> - add a pushbutton to clear the VTEterm. Also add a checkbox
> "Auto-clear on each run". For the occasional snippet that does
> terminal output, it can get confusing for the user to determine which
> is the latest output, especially if user is modifying code in the
> GTKSourceView.

Another contributor is actually working on clearing the terminal. :-)

> - shift the Exec/Copy/SaveAs buttonbar below the SourceView - it's a
> more logical place for it in the left-to-right, top-to-bottom flow for
> the UI: load code in the sourceview using the combo and treeview,
> (optionally) edit the code, hit Exec, see output in the VTETerm below.

Yeah, I have been thinking about this too. We may want to fix this.

> - see if I can (at runtime) add the shortcuts for the three buttons
> into their captions. Additionally, I'm thinking of changing the Copy
> button shortcut... when I want to copy only a portion of the snippet
> from the sourceview, I hit Ctrl-C and the button copies ALL the
> snippet to the clipboard. I've been bitten several times (and I added
> the shortcut! :-( ) -- I think other users might be annoyed too. Wish
> we had more feedback.

Aha, good idea.

So maybe we can fix these issues after you land the dep checker?

Thanks, Ed!

Revision history for this message
Ed S (edgar-b-dsouza) wrote :

On Sat, Mar 27, 2010 at 12:38 AM, Jono Bacon <email address hidden> wrote:
> Just so you know, I just released 0.4 (http://aciresnippets.wordpress.com/2010/03/26/acire-0-4-released/) - I may merge a few small changes in there and bug fixes: a few people have a few things that are working on.

Nice. :-)

> Bring it on!

OK, at some point in the future :)

> Another contributor is actually working on clearing the terminal. :-)

Ah - OK, good. I'm curious to see if s/he finds any other way than
resetting the terminal, which is all I could figure out.
Er - if I might ask - is there a mailing list, or IRC channel or
something where contributors discuss?

>> - shift the Exec/Copy/SaveAs buttonbar below the SourceView
> Yeah, I have been thinking about this too. We may want to fix this.

Not a big change, maybe it can be done even before the dep checker.

>> - see if I can (at runtime) add the shortcuts for the three buttons
> Aha, good idea.
> So maybe we can fix these issues after you land the dep checker?

If they're not done by then, yes, I will.

Revision history for this message
Jono Bacon (jonobacon) wrote :

> > Another contributor is actually working on clearing the terminal. :-)
>
> Ah - OK, good. I'm curious to see if s/he finds any other way than
> resetting the terminal, which is all I could figure out.

This is now committed: it was using vte.reset() - it clears the terminal before running the snippet and works nicely. :-)

As I said before: I recommend you keep pulling from lp:acire to ensure your branch is updated. I have another few bugfixes I need to merge in.

> Er - if I might ask - is there a mailing list, or IRC channel or
> something where contributors discuss?

Nope: the project is pretty small right now, so I think we can just do this in a place such as this until we have more contributors and set up a list.

> >> - shift the Exec/Copy/SaveAs buttonbar below the SourceView
> > Yeah, I have been thinking about this too. We may want to fix this.
>
> Not a big change, maybe it can be done even before the dep checker.

Personally, I would recommend you focus on the dep checker and not mix features: just makes it easier for us to ensure we get a clean implementation. :-)

> >> - see if I can (at runtime) add the shortcuts for the three buttons
> > Aha, good idea.
> > So maybe we can fix these issues after you land the dep checker?
>
> If they're not done by then, yes, I will.

Sweet. :-)

Revision history for this message
Ed S (edgar-b-dsouza) wrote :

On Sat, Mar 27, 2010 at 10:44 AM, Jono Bacon <email address hidden> wrote:
> Personally, I would recommend you focus on the dep checker and not mix features: just makes it easier for us to ensure we get a clean implementation. :-)

Will do :)

Is it OK to use python-apt? Will the dependency be added when
packaging for the PPA?

Revision history for this message
Jono Bacon (jonobacon) wrote :

Yep. :-)

Revision history for this message
Jono Bacon (jonobacon) wrote :

Hi Ed,

I just wanted to let you know that I am finding more and more bug fixes that I need to fix in Acire, so I expect lp:acire to change more frequently. I just wanted to make sure I gave you a heads up this to keep syncing your branch with lp:acire.

We discussed this before, but I would still recommend you keep the vast majority of your logic in a separate source file (e.g. depchecker.py) and then just hook it into bin/acire to integrate it into the app. This will mean that the regular changes in bin/acire will not screw up your branch. :-)

Thanks again for all your hard work, Ed!

Revision history for this message
Ed S (edgar-b-dsouza) wrote :

On Mon, Mar 29, 2010 at 1:26 AM, Jono Bacon <email address hidden> wrote:
> Hi Ed,
>
> I just wanted to let you know that I am finding more and more bug fixes that I need to fix in Acire, so I expect lp:acire to change more frequently. I just wanted to make sure I gave you a heads up this to keep syncing your branch with lp:acire.
>
> We discussed this before, but I would still recommend you keep the vast majority of your logic in a separate source file (e.g. depchecker.py) and then just hook it into bin/acire to integrate it into the app. This will mean that the regular changes in bin/acire will not screw up your branch. :-)

Thanks for the heads-up, Jono... I tried updating the other day, and
Bazaar Explorer tells me I'm already on a "divergent" branch :-P so
what I'm doing is just what you say - keep as much of the new code as
possible outside bin/acire and its .ui file. That will make merging
easier for me just before uploading for merge proposal. I'm still
refactoring/writing new code/testing and debugging, so merge proposal
is a few days away yet (had some other things take up time, so haven't
put in as much time as I'd like on Acire). My paying work cycle starts
on the 8th (to the 21st) so I intend to have this feature merged (or
at least proposed) by then, since I'll most likely be unable to work
on the project at all during those days.

Thanks,
Ed.

Revision history for this message
Jono Bacon (jonobacon) wrote :

> Thanks for the heads-up, Jono... I tried updating the other day, and
> Bazaar Explorer tells me I'm already on a "divergent" branch :-P so
> what I'm doing is just what you say - keep as much of the new code as
> possible outside bin/acire and its .ui file. That will make merging
> easier for me just before uploading for merge proposal. I'm still
> refactoring/writing new code/testing and debugging, so merge proposal
> is a few days away yet (had some other things take up time, so haven't
> put in as much time as I'd like on Acire). My paying work cycle starts
> on the 8th (to the 21st) so I intend to have this feature merged (or
> at least proposed) by then, since I'll most likely be unable to work
> on the project at all during those days.

This all sounds great, Ed. What would also be great to factor into your design and code is to possibly have a depchecker.py file which deals with the general tasks of checking which modules a snippet uses and detecting the OS, and then loading a separate .py file which has a standard set of methods for dealing with the logic of searching for and installing packages. This means that we could then open it up to other distros to simply provide a .py file appropriate to them.

Just a thought.

Thanks, Ed!

Revision history for this message
Ed S (edgar-b-dsouza) wrote :

On Mon, Mar 29, 2010 at 9:25 AM, Jono Bacon <email address hidden> wrote:
> This all sounds great, Ed. What would also be great to factor into your design and code is to possibly have a depchecker.py file which deals with the general tasks of checking which modules a snippet uses and detecting the OS, and then loading a separate .py file which has a standard set of methods for dealing with the logic of searching for and installing packages. This means that we could then open it up to other distros to simply provide a .py file appropriate to them.

:-) Already working on that, Jono - I found that the built-in platform
module's linux_distribution() function returns a tuple of which the
first is the distro name; using that, I'm writing a
"distropkgutils.py" in which I'm building a class that (in __init__)
will grab selected functions (defined in the file), based on the
distro, and expose those as its methods - so depchecker.py can simply
call methods of this class for all pkg-mgt-related work (including
retrieving a list of uninstalled packages, which I've hard-coded with
aptitude search now) without having to "know" about the underlying
package mgt mechanism. I saw your point, that it's way better to do
this the portable way right now, rather than get it into trunk and
then do several revisions to make it distro-portable.
Of course, the functions for distros other than Ubuntu will be a
"not_implemented" message stub, which other contributors can flesh out
later (my Fedora 12 VM crashed yesterday :-( so I'll just focus on
Ubuntu right now).

Revision history for this message
Jono Bacon (jonobacon) wrote :

Sounds great, Ed! :-)

Revision history for this message
Ed S (edgar-b-dsouza) wrote :

Jono, quick question:
To present the Firefox-style yellow bar with button, I propose to use an event box which contains an hbox inside which is a label and button.

I'd earlier put this into the Acire .ui file using Glade, but was wondering whether I could ask you to give a specific name to the vbox "vbox4" (which contains the editor viewport, docs_box etc), such as "right_pane_vbox" (or better name); and if I could just pass a reference to this vbox to depchecker.check_script() in snippet_selected() in bin/acire -- then build the partial UI mentioned above, on the fly. Would this be OK, or would you prefer to have all the UI components in the .ui file - i.e. added via Glade?

If the latter, are you still hacking bin/acire and its .ui file? When do you expect to push an updated version to trunk, that I can pull and apply changes to?

review: Needs Information
Revision history for this message
Ed S (edgar-b-dsouza) wrote :

Jono, hope you have some time to review the new branch (lp:~edgar-b-dsouza/acire/snippet_dependency_res_2 ) which I pushed now, since Bazaar Explorer refused to push to this branch.

Unmerged revisions

44. By Ed S

First-cut implementation of snippet dependency detection and resolution. Still needs attention to querying LP for PPAs to add to system repos.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: