aacirino / workspacemechanic

Automatically exported from code.google.com/p/workspacemechanic
0 stars 0 forks source link

Remote tasks (specified by URL) does not work #100

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Create task file and put on remote server (e.g. http://some/task.epl)
2. Create json descriptor, e.g.:
{
  type : 'com.google.eclipse.mechanic.UriTaskProviderModel',
  metadata : {
    name: 'Some',
    description: 'Some',
    contact: 'Someone'
  },  
  tasks: [
    'http://some/task.epl'
  ]
}
3. Place JSON on remote server (e.g. http://some/tasks.json)
4. In Workspace Mechanics add entry: http://some/tasks.json
5. Run Workspace Mechanics to check all tasks

Tasks defined by http://some/tasks.json will be never executed.

I've debugged code and I've noticed that potentially method 
com.google.eclipse.mechanic.LastModifiedPreferencesFileTask#evaluateByMD5() is 
missing situation when remote file have never been processed.
In this situation previous variable is always 0 (MD5 hash) and is always 
DIFFERENT than current MD5 hash (current variable). As a result 
com.google.eclipse.mechanic.LastModifiedPreferencesFileTask#evaluate() always 
returns true which means that task will be marked as TaskStatus.PASSED.

On the other hand adequate implementation for local files 
(com.google.eclipse.mechanic.LastModifiedPreferencesFileTask#evaluateByModificat
ionDate() method) distincts situation when file is process by the very first 
time:
line 113: return previous > 0L && ...

Original issue reported on code.google.com by Iso.poc...@gmail.com on 25 Oct 2012 at 8:14

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
I think that method 
com.google.eclipse.mechanic.LastModifiedPreferencesFileTask#evaluateByMD5()shoul
d not return:
previous != current

but
previous == current

Which means if hashes are equal then Task Passes. I've tested this fix and 
works like a charm. Detects new task file on remote server and it changes.

Please, let me know when you read this bug report.

Original comment by Iso.poc...@gmail.com on 25 Oct 2012 at 8:33

GoogleCodeExporter commented 9 years ago
Hmm... this seems reasonable and worth fixing. I wonder how this was missed.

I'm on vacation for two weeks and will address this upon my return.

Original comment by konigsb...@gmail.com on 25 Oct 2012 at 1:59

GoogleCodeExporter commented 9 years ago
Sorry to bother you, but I need to know, if you're still willing to fix this 
issue? Your plugin would be really helpful in our project and we would 
appreciate it to use the URL feature.

Original comment by rooon...@gmail.com on 26 Nov 2012 at 9:42

GoogleCodeExporter commented 9 years ago
I too would like to vote for this defect.

The implementation of Workspace Mechanic would significantly improve how 
Eclipse is managed in my organization, however this issue is holding up the 
deployment.

Original comment by riley.ti...@gmail.com on 29 Nov 2012 at 12:29

GoogleCodeExporter commented 9 years ago
I've been really slammed. Thanks for the reminder. Yes, I'll get to this this 
week, I promise.

Original comment by konigsb...@gmail.com on 29 Nov 2012 at 1:11

GoogleCodeExporter commented 9 years ago
As a temporary workaround I suggest to change task type to RECONCILE.

Original comment by Iso.poc...@gmail.com on 29 Nov 2012 at 1:33

GoogleCodeExporter commented 9 years ago
I am not certain the suggested workaround is valid and here is why.

Using the example from above 'http://some/task.epf', in my case it is set to 
RECONCILE and it still is not executed. The problem as far as I can tell is 
http://some/tasks.json is not being executed and neither are any of the tasks 
listed. So it is possible the LastModifiedPreferencesFileTask is being used for 
URL tasks to determine if a the JSON files have changed.

Disclaimer: I am not very familiar with the codebase so please excuse any 
mis-statements I may make.

Sincerely,
Tim

Original comment by riley.ti...@gmail.com on 29 Nov 2012 at 6:35

GoogleCodeExporter commented 9 years ago
This workaround works for me and for my team.
We're using Eclipse Indigo 3.7.2 with Workspace Mechanics 0.3.3.

Did you restore your workspace's preferences to defaults that workspace 
mechanics may actually works ? Note that RECONCILE tasks compares your current 
settings with those defined in *.epf task files. If there is no difference 
nothing will happen.

Original comment by Iso.poc...@gmail.com on 29 Nov 2012 at 7:36

GoogleCodeExporter commented 9 years ago
I've pushed to the testing repository. I did quite a bit of refactoring to make 
code testable, so your feedback will be valuable.

Original comment by konigsb...@gmail.com on 30 Nov 2012 at 1:35

GoogleCodeExporter commented 9 years ago
"Testing repository" -> do you mean 
https://code.google.com/a/eclipselabs.org/p/workspacemechanic/ ?

Original comment by Iso.poc...@gmail.com on 30 Nov 2012 at 10:36

GoogleCodeExporter commented 9 years ago
I've tested code from 
https://code.google.com/a/eclipselabs.org/p/workspacemechanic/ and it works 
like a charm.

I've some remarks to the code:

1. com.google.eclipse.mechanic.Evaluator interface's name is not 
self-describing to me. Moreover even more misleading is its method: evaluate(). 
For me this name suggest that this  method should do some logic without 
returning any value. I was really surprised when I discovered what method 
LastModifiedPreferencesFileTask#evaluate() do. It returns true if task does not 
need further evaluation (here it is right place for usage of this word) and 
false otherwise (reverse logic).
I think LastModifiedPreferencesFileTask#evaluate() should be renamed to 
something like: isApplicable() to be more self-describing (I know that this 
change requires further refactoring).

2. Methods evaluateByMD5 and evaluateByModificationDate has also ugly and 
misleading names.

3. RepairAction interface has been introduced only for its name. I think this 
broke one of the OOP basic rules.

To sum up, thanks for bug fix ! :)

Original comment by Iso.poc...@gmail.com on 30 Nov 2012 at 2:33

GoogleCodeExporter commented 9 years ago
Thanks for the feedback. Since the project is in maintenance mode, I'm not 
giving the 'bad names' feedback too much weight. Perhaps when Workspace 
Mechanic comes out of maintenance mode that can be addressed.

Original comment by konigsb...@gmail.com on 30 Nov 2012 at 3:30

GoogleCodeExporter commented 9 years ago
Thank you so much for taking the time to fix this issue so quickly.

Is there a deployment schedule or a time frame when to expect a 0.3.4 on the 
update site? 
http://workspacemechanic.eclipselabs.org.codespot.com/git.update/mechanic

Original comment by riley.ti...@gmail.com on 30 Nov 2012 at 8:32

GoogleCodeExporter commented 9 years ago
I'd like to get a couple more people report that it works, including Googlers 
that rely on this, whom I have asked outside this bug. But suffice it to say 
that it should be pushed some time in the first half of next week.

Original comment by konigsb...@gmail.com on 30 Nov 2012 at 9:23

GoogleCodeExporter commented 9 years ago
Works like a charm once I got the JSON correct.

I'd suggest updating the JSON example on the URLTasks 
(http://code.google.com/a/eclipselabs.org/p/workspacemechanic/wiki/URLTasks) 
wiki page.

Something like...
{
    "type": "com.google.eclipse.mechanic.UriTaskProviderModel",
    "metadata": {
        "description": "my set of tasks",
        "name": "Jane Programmer",
        "contact": "jane@projectfoo.com"
    },
    "tasks": [
        "http://www.projectfoo.com/mechanic/task1.epf",
        "http://www.projectfoo.com/mechanic/task2.epf",
        "relative-to-me.epf"
    ]
}

Thanks again for the quick turnaround.

Original comment by riley.ti...@gmail.com on 8 Dec 2012 at 12:51

GoogleCodeExporter commented 9 years ago
This has been pushed to the main branch.

Timothy, thanks for the suggestion. I'll look at it right now and apply it if 
it makes sense.

Original comment by konigsb...@gmail.com on 13 Dec 2012 at 11:15