Bug 279748 - [Progress] Job with property KEEP_PROPERTY doesn't keep if status not Status.OK_STATUS
Summary: [Progress] Job with property KEEP_PROPERTY doesn't keep if status not Status....
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.5   Edit
Hardware: PC Windows 2000
: P3 normal (vote)
Target Milestone: 3.7 M1   Edit
Assignee: Krzysztof Daniel CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-06-10 03:08 EDT by Alexey Markevich CLA
Modified: 2010-06-29 10:48 EDT (History)
5 users (show)

See Also:


Attachments
Patch v01 (1.07 KB, patch)
2009-09-01 04:14 EDT, Prakash Rangaraj CLA
no flags Details | Diff
Fix proposal (not tested) (2.42 KB, patch)
2009-09-01 05:17 EDT, Krzysztof Daniel CLA
no flags Details | Diff
Fix proposal v02 (5.26 KB, patch)
2010-01-28 09:28 EST, Krzysztof Daniel CLA
no flags Details | Diff
Fix with a test (7.84 KB, patch)
2010-06-29 10:17 EDT, Krzysztof Daniel CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Markevich CLA 2009-06-10 03:08:39 EDT
Sample code:
Job job = new UploadJob((IStructuredSelection) selection);
job.setProperty(IProgressConstants.KEEP_PROPERTY, Boolean.TRUE);
job.setUser(true);
job.schedule();

If job successed then it keeps in Progress view; otherwise if IStatus.ERROR returned then it doesn't keep.
Comment 1 Krzysztof Daniel CLA 2009-06-10 05:45:35 EDT
This is intentional. Jobs are removed after the error dialog is presented. Do you think it should be changed? How do you imagine that?
Comment 2 Alexey Markevich CLA 2009-06-10 05:57:56 EDT
I expected to see done job marked as failed but present in Progress list
Comment 3 Prakash Rangaraj CLA 2009-06-10 06:35:27 EDT
(In reply to comment #2)
> I expected to see done job marked as failed but present in Progress list

  I too feel that should be the right way to go. If the KEEP_PROPERTY is set, then we should be showing it in the view, whether returned status is ok/error. Probably we may additionally need an icon to distinguish the status.
Comment 4 Renat Zubairov CLA 2009-06-26 08:19:11 EDT
Hi.

As I can see this bug is targeted for 3.6 and not even 3.5 MR?
Would the patch make things happening faster?

Renat
Comment 5 Prakash Rangaraj CLA 2009-06-26 08:44:41 EDT
(In reply to comment #4)
> Hi.
> 
> As I can see this bug is targeted for 3.6 and not even 3.5 MR?
> Would the patch make things happening faster?
> 
> Renat
> 

I'm not sure whether to treat this as a bug/enhancement. And usually enhancements won't go into maintenance releases. Either case a patch would definitely speed up things :-)

Paul,
   Should we take this up for 3.5.1?
Comment 6 Paul Webster CLA 2009-06-26 10:11:28 EDT
(In reply to comment #5)
> 
> Paul,
>    Should we take this up for 3.5.1?

We would probably wait to address this properly in 3.6, since we need to think on the best way to identify a job is finished with an error.

PW


Comment 7 Prakash Rangaraj CLA 2009-09-01 04:14:53 EDT
Created attachment 146161 [details]
Patch v01
Comment 8 Krzysztof Daniel CLA 2009-09-01 04:20:50 EDT
I think your patch will keep also jobs without KEEP_PROPERTY, right?
Comment 9 Prakash Rangaraj CLA 2009-09-01 04:58:25 EDT
(In reply to comment #8)
> I think your patch will keep also jobs without KEEP_PROPERTY, right?

  Ouch. My bad. Looks like I need some sleep :(
Comment 10 Krzysztof Daniel CLA 2009-09-01 05:17:32 EDT
Created attachment 146169 [details]
Fix proposal (not tested)

Method removeErrorJobs was introduced for status handling. I think it is safe to modify it a little bit and make it to react on KEEP_PROPERTY properly.
Comment 11 Prakash Rangaraj CLA 2009-09-02 04:17:21 EDT
(In reply to comment #10)
> Created an attachment (id=146169) [details]
> Fix proposal (not tested)
> 
> Method removeErrorJobs was introduced for status handling. I think it is safe
> to modify it a little bit and make it to react on KEEP_PROPERTY properly.

   The patch looks good to me. Thanks Christopher.
Comment 12 Krzysztof Daniel CLA 2009-09-02 07:37:04 EDT
I have tested my patch and I am not quite satisfied.

KEEP_PROPERTY works fine (the job appears in the ProgressView), however if you press ProgressAnimationItem (the little progress marker in bottom-right workbench corner) the error dialog will appear and the job will be removed from progress view.

On the other hand, keeping that job longer will cause the error will appear using any possible occassion (other errors etc).

Not sure which is expected.
Comment 13 Krzysztof Daniel CLA 2009-09-02 07:39:51 EDT
To solve this bug correctly (keep the job in progress view and present error dialog only one I'll need more time).
Comment 14 Remy Suen CLA 2010-01-20 07:54:46 EST
This was targeted for M2. That ship has sailed. Please retarget accordingly.
Comment 15 Krzysztof Daniel CLA 2010-01-28 09:28:32 EST
Created attachment 157520 [details]
Fix proposal v02

Slightly better patch, in which status is presented only once per job.
Comment 16 Krzysztof Daniel CLA 2010-05-07 05:51:17 EDT
no time in 3.6, moving to 3.7
Comment 17 Krzysztof Daniel CLA 2010-06-29 10:17:06 EDT
Created attachment 173002 [details]
Fix with a test
Comment 18 Krzysztof Daniel CLA 2010-06-29 10:48:35 EDT
The test has not been released as it would suffer from the same problem as 288358