-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Wikimedia Commons extension doesn't appear in 3.8.0 #6581
Comments
I believe the same issue is seen in https://phabricator.wikimedia.org/T363917 I've rolled back to 3.7.9 in PAWS and the issue resolved. |
@sebastian-berlin-wmse @lokal-profil how does this issue fit within your development plans? Do you intend to work on it or do you see this as low priority (or out of scope)? If any fix is required on OpenRefine's side I would be keen to include it in a 3.8.1 release, which should ideally come out soon given that we have other important fixes to release (see #6599). If you don't intend to work on this soon I would try to find other ways to fix it (such as doing it myself). |
I'll have a look. |
For me, the extension only shows up in 3.7.7 and no later version (tried 3.7.8, 3.7.9 and 3.8.0). @wetneb, can you point me in the direction of where extensions are loaded so I can debug? I looked around a bit in the documentation and in the code, but I haven't found a good starting point. |
On the frontend side, extensions are loaded by including whatever Javascript files they declare into the page ( To debug the frontend you could look at the developer console to see if any errors appear there, or put breakpoints on the extension JS code to see if/how it gets executed. To debug the backend code you could first look at the server logs to see if any errors appear there. Running OpenRefine to debug an extension in Java is more involved (I am not sure how to set it up) but I generally work by writing unit tests which reproduce the bug and then debug the code via the IDE when running those tests. We can also look at this together in our call on Thursday. |
Turns out that my issue was because I misunderstood the instructions for installing extensions. I put commons-extension/ in extensions/ in OpenRefine root rather than in webapp/extensions/. I think I was confused by the screenshot under Install this extension in OpenRefine, which I now realise is of the workspace directory. Now the source for Commons shows up in 3.7.9, but not 3.8.0. |
I've managed to track down that 0945595 is the revision Commons stops showing up in the list of sources. It's a small change that only updates the Velocity library. |
When loading the Commons extension just after this commit, I get:
|
So after a joint investigation in our weekly call with @sebastian-berlin-wmse, we found out that the problem is coming from the presence of two incompatible copies of the velocity library in the classpath, following #6190. While #6190 updated velocity in OpenRefine, it did not update velocity in Butterfly, meaning that we ship both The upgrade was motivated by a vulnerability in velocity, which does not actually have implications for OpenRefine since our velocity templates are not user-controlled, so I would be tempted to first release a 3.8.2 with this PR reverted, and then work on upgrading velocity in Butterfly and shipping this upgrade in a later version. |
I actually do not understand why Velocity should be a dependency of OpenRefine itself. I think it should intuitively be a dependency of Butterfly only, because I can't recall anywhere where we use it in OpenRefine. I have tried removing the dependency in OpenRefine and OR seems to still work fine without (I haven't tried running the Cypress test suite though). However, reverting #6190 (be it by downgrading velocity or removing it entirely) does not seem to make the Commons extension work again. Although it would still be interesting to understand the cause of the incompatibility, I think it would also be worth updating the version of OpenRefine that the Commons extension is built against: |
This is why we need better isolation for extension dependencies. I'll try to have a look at what's going on sometime over the long weekend. |
I tried changing the version of OR for the extension to 3.8.1 and it doesn't compile:
|
I fixed the compile errors above using the suggestions provided by my IDE. Unfortunately, after that worked I got the same error as in #6581 (comment). |
Although it "only" updates Velocity, it's a major upgrade from 1.5 to 2.3. Apparently the error handling has changed. I'm still investigating to see exactly how it changed and how we need to compensate for it. |
I suspect the error in #6581 (comment) is caused by stale The change in resource loading behavior actually happened between Velocity 1.5, which is a dependency of OpenRefine 3.7.7, and Velocity 1.6.3 which is what the version of Butterfly was bundling at that point. I thought perhaps we could just depend on the Butterfly version, but it has the problem as well. We would need to revert to Velocity 1.5 to fix the immediate problem (or tell extension writers to check their extensions for this problem). Longer term (i.e. post 3.8.*) we probably want to have OpenRefine use Butterfly's version of Velocity so that we only have a single version. |
The compilation errors in #6581 (comment) are worrying because they seem to indicate incompatible API changes. That deserves more investigation. Similarly, I came across #6375 while debugging and we should be sure to warn extension developers about those removed dependencies in case they have undeclared dependencies on them. |
Folks who upgraded to 3.8.0 have lost the Wikimedia Commons extension. It's also gone on Wikimedia PAWS.
I have tried uninstalling and reinstalling the extension and it still doesn't appear.
The text was updated successfully, but these errors were encountered: