Michal Pryc wrote:
> Brock Pytlik wrote:
>   
>> Michal Pryc wrote:
>>     
>>> Brock,
>>> My bad, I've checked everything again and it is fine. Somehow I managed
>>> to install new API without api.py and api_errors.py on a fresh
>>> OpenSolaris 2008.5.
>>>
>>>
>>>       
>> Ah, no worries. I was pretty sure it was working since I could build
>> both for testing and the packages we deliver as well.
>> Glad every thing's ok. I'm looking forward to seeing the webrev for your
>> changes.
>>     
> Brock,
> The webrev is at:
>
> http://cr.opensolaris.org/~migi/06_10_2008_proposed_changes_to_consume_new_api_v1/
>
> This webrev should be applied on top of yours:
> http://cr.opensolaris.org/~bpytlik/ips-api-v7/
>
> Also this webrev consists of my previous changes which are not in the 
> gate but were send to review + changes to fit xDesign wireframes (new 
> icons, text on labels etc...).
>
> I am currently testing those bits so there will be another webrev when 
> everything will be polished, currently I am caching and raising errors 
> from the API but the final version will have proper error dialog.
>
> So in short three more things needs to be done from my side:
> - smoke test for all operations
> - pylint
> - dialogs for errors
>
> best
> Michal
>   
inatallupdate.py:
74-76: i don't understand why installupdate is creating its own api 
object instead of using the one created in packagemanager.py. There 
should really only be one api object per image in existence, doing 
otherwise could cause "bad things" to happen. (At the very least, having 
multiple api objects defeats all the locking which is in the api)

199: I'm not certain, but I would've guessed that api.cancel should be 
called here rather than api.reset.

219, 224, 319, 345: commented lines

290-295: as noted below, I hope to provide an api so that this is 
unnecessary, but it looks fine for now

459-460: I think this can be simplified as plan = 
api.describe().get_changes()



imageplan.py:
85: please split this list into two separate variables

522-538: this  needs to be done here so you can present the user with 
download sizes?


progress.py:
please resync with the gate/v8


packagemanager.py:
lines 353, 362: remove commented lines

1064: some typos on this line I think

1070: why not use the API info method?

1194-1208: I hope to provide an interface to update_all which eliminates 
the need for this, but for now this looks good, except that plan_install 
returns True or False, not 1 or 0 (this might be one of the late changes 
that went into v8)

1311-1328: find_root no longer raises a ValueError, it can raise an 
api_errors.ImageNotFoundException, please don't change the changes what 
were made here in changeset be1ad23e7704 unless there are good reasons

1730: I believe _get_version_installed has been removed from the code base

1794-1796: I'd guess, like with the update manager, you want to pass in 
a callback as the fourth argument which will get  called when the state 
of cancelability for the api changes

Brock

>   
>> Brock
>>     
>>> best
>>> Michal
>>>
>>>
>>>
>>> Brock Pytlik wrote:
>>>
>>>       
>>>> Michal Pryc wrote:
>>>>
>>>>         
>>>>> Brock,
>>>>> One minor thing which I didn't mention before. The setup.py and other
>>>>> build files were not modified to add the new files (api.py, ...) to
>>>>> the packages.
>>>>>
>>>>>           
>>>> Sorry, you're going to have to be clearer on that. What changes to
>>>> setup.py and build files do I need to make?
>>>>
>>>> Brock
>>>>
>>>>         
>>>>> regards
>>>>> Michal Pryc
>>>>>
>>>>> Brock Pytlik wrote:
>>>>>
>>>>>           
>>>>>> Michal Pryc wrote:
>>>>>>
>>>>>>             
>>>>>>> Brock,
>>>>>>> I've seen new version ips-api-v7 which looks fine for me.
>>>>>>>
>>>>>>> Is it possible to push those changes to the gate Today?
>>>>>>>
>>>>>>>
>>>>>>>               
>>>>>> I'm waiting on a final code review from the people over here. I'll make
>>>>>> the rounds and see what kind of commitment I can get.
>>>>>>
>>>>>> Brock
>>>>>>
>>>>>>             
>>>>>>> best
>>>>>>> Michal
>>>>>>>
>>>>>>>
>>>>>>> Brock Pytlik wrote:
>>>>>>>
>>>>>>>               
>>>>>>>> v6 fixes a few typos and fixes a problem where canceling an operation
>>>>>>>> resulted in a deadlock.
>>>>>>>> http://cr.opensolaris.org/~bpytlik/ips-api-v6/
>>>>>>>>
>>>>>>>> This version passes all of the test suite. When put back, it will
>>>>>>>> temporarily break removing packages using packagemanager. That will be
>>>>>>>> resolved will the GUI gets updated to use the API, something I
>>>>>>>> understand is being targeted for put back late this week. For this,
>>>>>>>> and
>>>>>>>> many other reasons, I'm targeting the beginning of 100 for this
>>>>>>>> putback,
>>>>>>>> and not the tail end of 99.
>>>>>>>>
>>>>>>>> Brock
>>>>>>>>
>>>>>>>> Brock Pytlik wrote:
>>>>>>>>
>>>>>>>>                 
>>>>>>>>> v5 changes 6 lines, 147-148, 200-201, 310-311. This simply makes the
>>>>>>>>> no-execute flag/do nothing argument make sure that prepare and
>>>>>>>>> execute_plan cannot be called on the api_obj if the -n option is
>>>>>>>>> given.
>>>>>>>>> Shawn's webrev inspired me to make sure this functionality was
>>>>>>>>> correct.
>>>>>>>>>
>>>>>>>>> http://cr.opensolaris.org/~bpytlik/ips-api-v5
>>>>>>>>>
>>>>>>>>> Brock
>>>>>>>>>
>>>>>>>>> Brock Pytlik wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>                   
>>>>>>>>>> v4 is simply a resync with the gate:
>>>>>>>>>> http://cr.opensolaris.org/~bpytlik/ips-api-v4/
>>>>>>>>>>
>>>>>>>>>> Brock Pytlik wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>                     
>>>>>>>>>>> Another new version is out. For the CLI and backend it's
>>>>>>>>>>> essentially
>>>>>>>>>>> unchanged. It introduces changes to the GUI code to help it not
>>>>>>>>>>> break
>>>>>>>>>>> when the API is put back.
>>>>>>>>>>>
>>>>>>>>>>> http://cr.opensolaris.org/~bpytlik/ips-api-v3/
>>>>>>>>>>>
>>>>>>>>>>> John, Michal, could you please take a look at the changes to the
>>>>>>>>>>> GUI
>>>>>>>>>>> code in general, and the changes to remove.py in particular.
>>>>>>>>>>> Somehow my
>>>>>>>>>>> changes seem to make the GUI never realize it's done evaluating the
>>>>>>>>>>> removal plan, and I'm lost as to why remove would be showing this
>>>>>>>>>>> behavior when install and image-update work. From looking at the
>>>>>>>>>>> truss
>>>>>>>>>>> output, it seems like it spends time reading files, then finishes,
>>>>>>>>>>> and
>>>>>>>>>>> just spins, updating progress bar.
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>> Brock
>>>>>>>>>>>
>>>>>>>>>>> Brock Pytlik wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>                       
>>>>>>>>>>>> New webrev reflecting the comments I've gotten so far, and
>>>>>>>>>>>> resyncing
>>>>>>>>>>>> with the gate to gather in the new changes.
>>>>>>>>>>>>
>>>>>>>>>>>> http://cr.opensolaris.org/~bpytlik/ips-api-v2/
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>> Brock
>>>>>>>>>>>>
>>>>>>>>>>>> Brock Pytlik wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>                         
>>>>>>>>>>>>> Here's the link:
>>>>>>>>>>>>> http://cr.opensolaris.org/~bpytlik/ips-api-v1/
>>>>>>>>>>>>>
>>>>>>>>>>>>> I haven't done as much cleanup as I usually try to do, but since
>>>>>>>>>>>>> I'm
>>>>>>>>>>>>> ooto tomorrow, I thought it would be better to get this out now,
>>>>>>>>>>>>> rather
>>>>>>>>>>>>> than next Monday.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Other things that I know need to be improved, but that I think
>>>>>>>>>>>>> can wait
>>>>>>>>>>>>> for another round of revisions:
>>>>>>>>>>>>> API Exception organization
>>>>>>>>>>>>> More complete functionality
>>>>>>>>>>>>> Automated testing of cancellation (I hope to work with the GUI
>>>>>>>>>>>>> team to
>>>>>>>>>>>>> test this by hand in the interim)
>>>>>>>>>>>>> Versioning of the Progress tracker (perhaps)
>>>>>>>>>>>>> More Documentation
>>>>>>>>>>>>> Figure out the future approach to testing we'll take
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>> Brock
>>>>>>>>>>>>> _______________________________________________
>>>>>>>>>>>>> pkg-discuss mailing list
>>>>>>>>>>>>> [email protected]
>>>>>>>>>>>>> http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>                           
>>>>>>>>>>>> _______________________________________________
>>>>>>>>>>>> pkg-discuss mailing list
>>>>>>>>>>>> [email protected]
>>>>>>>>>>>> http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>                         
>>>>>>>>>>> _______________________________________________
>>>>>>>>>>> pkg-discuss mailing list
>>>>>>>>>>> [email protected]
>>>>>>>>>>> http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>                       
>>>>>>>>>> _______________________________________________
>>>>>>>>>> pkg-discuss mailing list
>>>>>>>>>> [email protected]
>>>>>>>>>> http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>                     
>>>>>>>>> _______________________________________________
>>>>>>>>> pkg-discuss mailing list
>>>>>>>>> [email protected]
>>>>>>>>> http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>                   
>>>>>>>> _______________________________________________
>>>>>>>> pkg-discuss mailing list
>>>>>>>> [email protected]
>>>>>>>> http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
>>>>>>>>
>>>>>>>>                 
>>>> _______________________________________________
>>>> pkg-discuss mailing list
>>>> [email protected]
>>>> http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
>>>>
>>>>         
>>> _______________________________________________
>>> pkg-discuss mailing list
>>> [email protected]
>>> http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
>>>
>>>       
>> _______________________________________________
>> pkg-discuss mailing list
>> [email protected]
>> http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
>>     
>
> _______________________________________________
> pkg-discuss mailing list
> [email protected]
> http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
>   

_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to