Community
Participate
Working Groups
https://gerrit-documentation.storage.googleapis.com/ReleaseNotes/ReleaseNotes-2.11.html
Created attachment 252739 [details] Error from Mylyn Gerrit Connector using Gerrit 2.11
Just wanted to report a finding from testing Mylyn 3.15 against Gerrit 2.11. An error (see attached text file) occurs when adding a new query or opening a review task. The symptom is similar to what I experienced when testing Mylyn 3.14 against Gerrit 2.10. In particular, the newer version(s) of Gerrit seems to have introduced an extra field called isNoteDbEnabled which the Mylyn Gerrit Connector cannot process.
Thanks! It looks like a problem with the way we're extracting JSON from the JavaScript response.
Created attachment 252746 [details] mylyn/context/zip
The string we're trying to interpret as JSON is a JSON object followed by a comma, followed by "isNoteDbEnabled":false', so it looks like there may be an outer set of braces that we are stripping off before we call Gson.
Correct. In GerritHtmlProcessor.parse(), we split the Json string into tokens. The first token, until now, has been of the form '{ ... json data we want ...}}' so there was a line of code to remove the last character of the token before JSON parsing. Because of the isNoteDbEnabled field, the first token is now of the form '{... some json we want ...} ...isNoteDbEnabled}' so removing the last character leaves us with un-parsable json. I've made a quick fix https://git.eclipse.org/r/#/c/46721/ in which we remove everything after the second last curly bracket, though this is likely just as fragile. It might take some time to rewrite the parse method though as there's a bit of regular expression trickery going on. I also want to investigate a bit more into what this new isNoteDbEnabled field is. There is no information about it. Only 3 items come up in a google search...
I think we can probably ignore isNoteDbEnabled. It would be great if you could run the puppet script locally and run the tests.
> though this is likely just as fragile Yes, what you a trying to parse here is a Gerrit internal data structure and other tools should not rely on it. It's not a stable API. Which exact information from this config do you need? Maybe Gerrit could expose this information over the REST API so that in future you do not need to parse this data structure anymore. > I also want to investigate a bit more into what this new isNoteDbEnabled > field is. There is no information about it. Gerrit is moving the storage of its meta data (changes, comments, approvals etc) from the database into the git repositories where they are stored as git notes. This new persistence is called notedb. This is still development in progress and it's not ready for productive use. This is why it isn't documented yet. > I think we can probably ignore isNoteDbEnabled. Yes, for now you can savely ignore this field.
Thanks for the response Edwin, I can tell you which fields we're collecting from this internal data structure, but I haven't done any technical analysis to see which fields we actively use (but could be avoided), which ones are critical, and which ones aren't even touched once collected. Config items collected: allowRegisterNewEmail, authType, documenationAvailable, downloadSchmes, editable AccountFields, sshdAddress, useContactInfo, useContributorAgreements, wildProject
Thanks Nicholas for providing this info. I've now pushed a change to Gerrit master that adds a new REST endpoint that provides the information you need via a stable API [1]. If accepted it will be only released with Gerrit 2.12, but at least from then on it should prevent another such breakage. Please have a look at the change and say if you need anything more. Please note that the new REST endpint doesn't provide the SSHD address and whether documentation is available or not. Both cannot be easily provided without messing around with Gerrit Guice bindings. However you can easily get both information already now: - SSHD address: Use the '/ssh_info' URL for this, e.g. see [2]. - documenationAvailable: make a HEAD request to '/Documentation/index.html' and check whether you get OK or NOT FOUND, e.g. see [3]. [1] https://gerrit-review.googlesource.com/67572 [2] https://git.eclipse.org/r/ssh_info [3] https://git.eclipse.org/r/Documentation/index.html
Forget to say that 'allowRegisterNewEmail' is never set and I proposed to remove this field [1]. [1] https://gerrit-review.googlesource.com/67568
Based on a quick browse through the code and documentation file it looks great. I've submitted a new bug report to use the new REST endpoint in the Connector when Gerrit 2.12 becomes available. https://bugs.eclipse.org/bugs/show_bug.cgi?id=465971
All tests passed: https://hudson.eclipse.org/hudson/job/mylyn-reviews-nightly/3510/ Gerrit 2.11 is now supported!
Hey, that's great! Thanks!
Off Topic, do you know about the Gerrit REST Java client [1]? It is a client-side Java library to access the Gerrit REST API. It's used as base for the Gerrit IntelliJ Plugin [2]. Have you considered to use it as base for Gerrit Mylyn Connector as well? I think this would make a lot of sense. If you are interested, at best contact Urs Wolfer, the maintainer of this client lib. [1] https://github.com/uwolfer/gerrit-rest-java-client [2] https://github.com/uwolfer/gerrit-intellij-plugin
Thanks for the info. I had not heard of that. It might be worth looking into that, but I've found in the past that relying on these kinds of toolkits can be problematic because they often don't support everything that we need in Mylyn, such as progress monitoring and cancellation of requests.
@Sam Davis: Any pull requests for missing features are very welcome. If you have any questions, feel free to contact me. Things like progress tracking should be easily doable - I also need it in the IntelliJ Gerrit plugin.
Thanks very much. Does the java client depend (directly or indirectly) on com.google.gerrit.reviewdb?
No, it doesn't depend on com.google.gerrit.reviewdb. All communication is done via REST API. It implements the official Gerrit extensions interface (so the impl could be replaced with another impl (once there exists one) which implements this interface). The lib also supports many different auth methods (digest, http form, gerrit-auth-token, ...).
Thanks. I would like to do some cleanup of the Gerrit connector and that would be a good chance to think about adopting the Java client.
