Bug 220906 - Default location of workflow files
Summary: Default location of workflow files
Status: REOPENED
Alias: None
Product: z_Archived
Classification: Eclipse Foundation
Component: Geclipse (show other bugs)
Version: unspecified   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: Ashish Thandavan CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed
Depends on:
Blocks:
 
Reported: 2008-02-29 04:44 EST by Katarzyna Bylec CLA
Modified: 2014-01-09 16:15 EST (History)
3 users (show)

See Also:


Attachments
eu.geclipse.ui plugin.xml patch (1.77 KB, patch)
2008-07-01 18:11 EDT, David Johnson CLA
no flags Details | Diff
eu.geclipse.core.internal.model.GridProject patch (805 bytes, patch)
2008-07-01 18:14 EDT, David Johnson CLA
no flags Details | Diff
patch to fix default folder in New Workflow wizard (3.48 KB, patch)
2008-10-06 13:33 EDT, David Johnson CLA
aog-ecl: iplog+
Details | Diff
patch for GridProject to fix default Workflow folder (540 bytes, patch)
2008-12-17 10:27 EST, David Johnson CLA
aog-ecl: iplog+
Details | Diff
Patch to fix the default wf folder (788 bytes, patch)
2008-12-26 14:14 EST, Mathias Stümpert CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Katarzyna Bylec CLA 2008-02-29 04:44:49 EST
When creating new workflow from project's context menu Wizard should propose to put workflow files in Workflows folder by default (see behavior of Grid Connection and Job Description Wizards - select any project in Grid Project View, and choose one of those wizards from context menu)
Comment 1 David Johnson CLA 2008-07-01 18:11:28 EDT
Created attachment 106263 [details]
eu.geclipse.ui plugin.xml patch

Part 1/2 of fix for this bug.
Comment 2 David Johnson CLA 2008-07-01 18:14:52 EDT
Created attachment 106264 [details]
eu.geclipse.core.internal.model.GridProject patch

Part 2/2 of fix for this bug.

A committer  please apply patches and bug reporter verify fix.
Comment 3 Ashish Thandavan CLA 2008-07-02 02:50:48 EDT
Does this take into account the fact that a 'Workflows' folder is not created by default for a Grid Project? (Or is that part of the fix?) 
(In reply to comment #2)
> Created an attachment (id=106264) [details]
> eu.geclipse.core.internal.model.GridProject patch
> 
> Part 2/2 of fix for this bug.
> 
> A committer  please apply patches and bug reporter verify fix.
> 

Does this take into account the fact that a 'Workflows' folder is not created by default for a Grid Project? (Or is that part of the fix?) 
Comment 4 Ashish Thandavan CLA 2008-07-02 02:51:19 EDT
CC-ing David
Comment 5 David Johnson CLA 2008-07-02 03:09:39 EDT
(In reply to comment #3)
> Does this take into account the fact that a 'Workflows' folder is not created
> by default for a Grid Project? (Or is that part of the fix?) 
> 

Good point... let me check!
Comment 6 David Johnson CLA 2008-07-04 07:02:11 EDT
> Does this take into account the fact that a 'Workflows' folder is not created
> by default for a Grid Project? (Or is that part of the fix?) 
> 

It seems to default to project root, so should be OK.
Comment 7 David Johnson CLA 2008-08-06 08:52:40 EDT
Things have changed since the proposed patches I put before were written, so this still needs fixing.
Comment 8 David Johnson CLA 2008-10-06 11:24:31 EDT
Comment on attachment 106263 [details]
eu.geclipse.ui plugin.xml patch

marking patch as obselete
Comment 9 David Johnson CLA 2008-10-06 11:24:45 EDT
Comment on attachment 106264 [details]
eu.geclipse.core.internal.model.GridProject patch

marking patch as obselete
Comment 10 David Johnson CLA 2008-10-06 13:33:27 EDT
Created attachment 114334 [details]
patch to fix default folder in New Workflow wizard

Adding an updated patch.

This patch makes the New Workflow wizard default the selected folder to 'Workflows' if it exists in Grid Project. If not, it defaults to 'Job Descriptions' folder.
Comment 11 Ashish Thandavan CLA 2008-10-07 11:03:27 EDT
(In reply to comment #10)
> Created an attachment (id=114334) [details]
> patch to fix default folder in New Workflow wizard
> 
> Adding an updated patch.
> 
> This patch makes the New Workflow wizard default the selected folder to
> 'Workflows' if it exists in Grid Project. If not, it defaults to 'Job
> Descriptions' folder.
> 

Applied, tested and committed. Works on Mac OS X and 32-bit Linux. Refused to work as expected on 64-bit Linux. Please check.
Comment 12 David Johnson CLA 2008-12-15 09:39:48 EST
Once AGAIN, it seems this bug has been UNfixed. Tested on Windows XP and it's defaulting to the Job Descriptions folder.
Comment 13 David Johnson CLA 2008-12-17 10:27:11 EST
Created attachment 120701 [details]
patch for GridProject to fix default Workflow folder

Added a patch that fixes the unfixed earlier fix.

Someone please commit on my behalf since I have no SVN access.
Comment 14 Ashish Thandavan CLA 2008-12-18 11:29:58 EST
(In reply to comment #13)
> Created an attachment (id=120701) [details]
> patch for GridProject to fix default Workflow folder
> 
> Added a patch that fixes the unfixed earlier fix.
> 
> Someone please commit on my behalf since I have no SVN access.
> 

applied and tested. Works on Mac and Windows but not on 64-bit Linux _with_ the patch. WithOUT the patch - works as expected on 64-bit Linux but does NOT work as expected on Windows and Mac (and possibly 32-bit Linux - needs re-checking!)

I've committed the patch anyway.
Comment 15 Ariel Garcia CLA 2008-12-26 10:07:51 EST
Ash, David, commenting out that "break;" statement in GridProject.java doesn't actually change anything, the same with Linux or Windows or 32/64 bits, the fact is:

with the "break;" the code is a tad more performant because it stops at the first element matching. Without the break you will keep the last match instead of the first one. And we are dealing there with extensions coming from an eclipse extension point... which you are not guaranteed that they will be always in the same ordering... actually they will not, and the order will not only change from one platform to the other but could also change between restarts, depending on the order in which the plugings where found/loaded etc.

The fact is that IGridWorkflowDescription
    extends IGridJobDescription
that's why  elementClass.isAssignableFrom( elementType )  in GridProject.java:168  also gives true for them, and why they land in the JD folder when there is no WF folder.

We should probably here also have some kind of priorities associated to the choices...
Comment 16 Mathias Stümpert CLA 2008-12-26 14:14:06 EST
Created attachment 121257 [details]
Patch to fix the default wf folder
Comment 17 Mathias Stümpert CLA 2008-12-26 14:19:12 EST
Reopening bug until the patch is confirmed ...

The patch prefers direct class matches over instanceof matches. That results in the following deterministic behaviour:

1) If a WF folder is present this is chosen by default
2) If no WF folder is present but a JD folder is this is chosen by default
3) If neither a WF folder nor a JD folder is present the default folder defaults to the project itself.

Actually I believe that it should exactly be like that since it is ok to let a WF go to the JD folder if no dedicated WF folder is present since the WF is nothing else than a JD.

Please someone verify, apply and close this bug if the solution is ok for everybody.

Concerning Ariel's suggestion with the priorities I don't think that makes sense at the current state. Maybe we have to think about that in the future but I honestly hope that our projects will never be cluttered with such masses o project folders that we need to have priorities assigned to them ;-)
Comment 18 Ariel Garcia CLA 2008-12-26 15:07:53 EST
Great, yes, that's the right fix imho. Patch applied.
David, Ash, could you please check in your different machines and close? Txs.
Comment 19 Ariel Garcia CLA 2009-05-03 11:13:49 EDT
Comment on attachment 114334 [details]
patch to fix default folder in New Workflow wizard

Applied by Ashish
Comment 20 Ariel Garcia CLA 2009-05-03 11:14:28 EDT
Comment on attachment 120701 [details]
patch for GridProject to fix default Workflow folder

Applied by Ashish