Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 347405

Summary:

[gerrit] Incompatibility with Gerrit 2.1.7-rc2-24-gb7ebfe1

Product:

z_Archived

Reporter:

Sascha Scholz <sascha.scholz>

Component:

Mylyn

Assignee:

Sascha Scholz <sascha.scholz>

Status:

RESOLVED FIXED

QA Contact:

Severity:

critical

Priority:

CC:

sop, steffen.pingel

Version:

unspecified

Target Milestone:

0.8

Hardware:

All

OS:

All

Whiteboard:

Attachments:

Description	Flags
mylyn/context/zip	none

Description Sascha Scholz

2011-05-27 03:18:31 EDT

There seems to be an incompatibility between the Gerrit connector and the latest Gerrit release candidate (2.1.7-rc2) probably due to a changed service interface. See https://review.source.android.com/#change,22769

org.eclipse.mylyn.internal.gerrit.core.client.GerritException: Error parsing request
               at org.eclipse.mylyn.internal.gerrit.core.client.JSonSupport.parseResponse(JSonSupport.java:155)
               at org.eclipse.mylyn.internal.gerrit.core.client.GerritService.invoke(GerritService.java:102)
               at $Proxy15.patchSetDetail(Unknown Source)
               at org.eclipse.mylyn.internal.gerrit.core.client.GerritClient$7.execute(GerritClient.java:358)
               at org.eclipse.mylyn.internal.gerrit.core.client.GerritClient.execute(GerritClient.java:660)
               at org.eclipse.mylyn.internal.gerrit.core.client.GerritClient.getPatchSetDetail(GerritClient.java:355)
               at org.eclipse.mylyn.internal.gerrit.core.client.GerritClient.getChange(GerritClient.java:379)
               at org.eclipse.mylyn.internal.gerrit.core.GerritTaskDataHandler.getTaskData(GerritTaskDataHandler.java:81)
               at org.eclipse.mylyn.internal.gerrit.core.GerritConnector.getTaskData(GerritConnector.java:123)
               at org.eclipse.mylyn.internal.tasks.core.sync.SynchronizeTasksJob.synchronizeTask(SynchronizeTasksJob.java:245)
               at org.eclipse.mylyn.internal.tasks.core.sync.SynchronizeTasksJob.runInternal(SynchronizeTasksJob.java:218)
               at org.eclipse.mylyn.internal.tasks.core.sync.SynchronizeTasksJob.run(SynchronizeTasksJob.java:153)
               at org.eclipse.mylyn.internal.tasks.core.sync.SynchronizeTasksJob.run(SynchronizeTasksJob.java:129)
               at org.eclipse.core.internal.jobs.Worker.run(Worker.java:54)

Comment 1 Steffen Pingel

2011-05-27 11:28:28 EDT

Args. Can you check what has changed in PatchSetDetail between 2.1.5 and the current release?

Comment 2 Sascha Scholz

2011-05-27 16:38:15 EDT

In ChangeSetDetailService

	void patchSetDetail(PatchSet.Id keyA, PatchSet.Id keyB, AccountDiffPreference diffPrefs, AsyncCallback<PatchSetDetail> callback);

was changed to

	void patchSetDetail(PatchSet.Id keyA, PatchSet.Id keyB, AccountDiffPreference diffPrefs, AsyncCallback<PatchSetDetail> callback);
	
keyB and diffPrefs can be null, leading to the old behaviour (tested locally).

The problem is (besides that Gerrit doesn't offer an API) that we don't have a mechanism in the GerritClient which allows us to invoke one or the other method depending on the Gerrit instance version (if we had the version information...).

Comment 3 Steffen Pingel

2011-05-28 11:12:07 EDT

Can you file a bug against Gerrit to ask to make this backwards compatible, i.e. provide the old method signature as well? 

The other thing that we could do as a work-around is to extend ChangeDetailService on the client side with the new method. Invocation wise we would try the first method and then fall-back to the new signature in case of an error. Does that make sense?

Comment 4 Sascha Scholz

2011-05-29 09:15:46 EDT

Created attachment 196846 [details]
mylyn/context/zip

Comment 5 Sascha Scholz

2011-05-29 09:19:52 EDT

Temporary workaround implemented in http://git.eclipse.org/c/mylyn/org.eclipse.mylyn.reviews.git/commit/?id=36b12bc21f8b134959268c2e2ee861b237f13fb8. Still needs to be clarified with Gerrit guys how to proceed.

Comment 6 Sascha Scholz

2011-06-01 10:22:01 EDT

Discussion on Gerrit group: http://groups.google.com/group/repo-discuss/browse_thread/thread/a142c3fe953d662e

Next steps:
- Propose bugfix to Gerrit
- Reverse logic for changelist service calls

Comment 7 Steffen Pingel

2011-06-06 11:42:52 EDT

Thanks Sascha. I'll mark this bug as resolved to ensure compatibility with the current Gerrit releases. We can revisit removing the work-around for 0.9.