-
-
Notifications
You must be signed in to change notification settings - Fork 323
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
g.extension: fix installing addons on the OS MS Windows #3166
g.extension: fix installing addons on the OS MS Windows #3166
Conversation
Without dependency on Git program. Gettings official GitHub addons repository branches is based on the parsing official GitHub addons repository all branches HTML page URL instead of using GitHub REST API due restrictions.
I tested it on the OS MS Windows 11 platform and it works as expected. Single addon installation e.g. db.join:
Multi addons installation e.g. i.sentinel:
List available extensions in the official GRASS GIS Addons repository:
|
Thanks for looking into it. It looks like with the changes in your PR It seems you disabled the post processing on Windows, so I was wondering what exactly is the consequence of that? |
Yes I agree is not "best" solution but we don't want use Git program and GitHub REST API. Reason why I use request/response technique (currently parsing official GitHub addons all branches HTML page URL) for getting all official GitHub addons branches is this condition inside grass/scripts/g.extension/g.extension.py Lines 523 to 524 in 6c69587
If we do not care about this condition in the OS Windows OS system, it may happen that the new release of the major version of GRASS GIS if the corresponding Git addons branch is not created, obtaining a list of available addons stops working (as it was in case of transition from major version from 7 to version 8).
Postprocessing addons man HTML page is done during compilation process on the server side. |
As far as I understand, the installation on Windows needs the binaries which are for specific minor version or release, so that's what ultimately decides if a user is able to install an addon. Listing addons is not really that important if one can't install them afterwards. The situation is different outside of Windows where the addon is compiled for all versions from the branch, so there, the existence of branch matters and, at the same time, falling back to a different branch produces a good result. It seems to me that on Windows, we need to assume that the server-side infrastructure is in place. It would be good to have 100% of g.extension on Windows even during v8 to v9 transition, but I don't think that's possible now and in any case, it does not need to be in 8.3.1 where we need small and safe solution. With the web scraping implementation, I would be concerned that it will break sooner due to changes in the web page then we come close to the v9 release where it would be actually useful (so for that changing manually whatever is needed when the time come would be an easier route, although not an ideal one). |
I reverted back code 9644e48 to using original solution GitHub REST API for getting official addons repository Git branches as compromise solution. I improve comprehensibility of GitHub REST API rate limit exceeded fatal message.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor wording fixes
How likely is it to exceed the rate limit? Since we already used the API and then got rid of it, could you clarify why this time it's not a problem? I haven't followed this at that time, so I want to make sure I understand it. I think part of the problem is that the code in general could be better structured, the We could create a PR for 8.3 releasebranch that would simply return the major version for Windows (plus the other changes in this PR) and leave this for further discussion so that this won't further delay the release. |
Currently, this PR only uses the GitHub REST API in one place in the GRASS GIS 8.2 used the GitHub REST API in several places during addon installation and post-processing of addon manual pages. This means that during the installation of one addon, 3-4 requests (i don't have exact number now) were made to the GitHub REST API server, with a limit of 60 requests per hour per computer IP address. Solution in GRASS GIS 8.3. was to replace the use of the GitHUB REST API with a Git program.
As I mentioned in my comment above. Reason why I use request/response technique (currently parsing official GitHub addons all branches HTML page URL) for getting all official GitHub addons branches is this condition inside grass/scripts/g.extension/g.extension.py Lines 523 to 524 in 6c69587
If we do not care about this condition in the OS Windows OS system, it may happen that the new release of the major version of GRASS GIS if the corresponding Git addons branch is not created, obtaining a list of available addons
But I agree with you, that for version 8.3.1 we can return simply the major version. |
the GitHub REST API is used major version branch name.
I incorporated a4ceb4c your suggestion that simply return the major version branch. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for all the effort. I can't easily test this now, but as long as you tested it on Windows, feel free to merge.
Without dependency on Git program.
Without dependency on Git program.
tested by
from the GUI
the xml file is on the server:
is this fix meant for 8.4? |
Your reported bug is not related with this PR and it is completely another bug. The bug is related to an SSL certificate validation error when trying to download an XML file from the URL https://grass.osgeo.org/addons/grass8/modules.xml and the error does not appear on a very MS Windows installation. The bug has already been reported in the past #2150. |
ah yes I remember that issue. I'll try on some other windows boxes later. |
how can we support users on not such recent windows boxes? |
Without dependency on Git program.
Without dependency on Git program.
Fixes #3077.
Fix installing addons via g.extension module on the OS MS Windows without dependency on Git program.
Gettings official GitHub addons repository branches
g.extension -l
is based on the parsing official GitHub addons repository all branches HTML page URL /~https://github.com/OSGeo/grass-addons/branches/all/ (request/response) instead of using GitHub REST API due restrictions.