Skip to content
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

Merged

Conversation

tmszi
Copy link
Member

@tmszi tmszi commented Sep 20, 2023

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.

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.
@tmszi tmszi added bug Something isn't working Windows Microsoft Windows specific Python Related code is in Python backport to 8.3 PR needs to be backported to release branch 8.3 backport to 8.2 PR needs to be backported to release branch 8.2 labels Sep 20, 2023
@tmszi tmszi added this to the 8.4.0 milestone Sep 20, 2023
@tmszi
Copy link
Member Author

tmszi commented Sep 20, 2023

I tested it on the OS MS Windows 11 platform and it works as expected.

Single addon installation e.g. db.join:

C:\Users\User>g.extension db.join
Downloading precompiled GRASS Addons <db.join>...
Fetching <db.join> from
<http://wingrass.fsv.cvut.cz/grass84/addons/grass-8.4.0dev/db.join.zip> (be
patient)...
Updating extensions metadata file...
Updating extension modules metadata file...
Installation of <db.join> successfully finished

C:\Users\User>db.join --help
Joins a database table to another database table.

Usage:
 db.join.py table=name column=name [database=name] [driver=name]
   other_table=name other_column=name [subset_columns=name[,name,...]]
   [--help] [--verbose] [--quiet] [--ui]

Parameters:
           table   Table to which to join other table
          column   Identifier column (e.g.: cat) in the table to be used for join
        database   Name of database
          driver   Name of database driver
                   options: dbf,odbc,ogr,sqlite,pg
     other_table   Other table name
    other_column   Identifier column (e.g.: id) in the other table used for join
  subset_columns   Subset of columns from the other table

Multi addons installation e.g. i.sentinel:

C:\Users\User>g.extension i.sentinel
Downloading precompiled GRASS Addons <i.sentinel>...
Fetching <i.sentinel> from
<http://wingrass.fsv.cvut.cz/grass84/addons/grass-8.4.0dev/i.sentinel.zip>
(be patient)...
Updating extensions metadata file...
Updating extension modules metadata file...
Installation of <i.sentinel> successfully finished

C:\Users\User>i.sentinel.download --help
Downloads Sentinel satellite data from Copernicus Open Access Hub, USGS Earth Explorer, or Google Cloud Storage.

Usage:
 i.sentinel.download.py [-lsb] [settings=name] [output=name]
   [footprints=name] [map=name] [area_relation=string] [clouds=value]
   [producttype=string] [start=string] [end=string] [limit=value]
   [query=string] [uuid=string[,string,...]] [relativeorbitnumber=value]
   [sleep=value] [retry=value] [datasource=value] [sort=value[,value,...]]
   [order=value] [--overwrite] [--help] [--verbose] [--quiet] [--ui]

Flags:
  -l   List filtered products and exit
  -s   Skip scenes that have already been downloaded after ingestiondate
  -b   Use the borders of the AOI polygon and not the region of the AOI

Parameters:
             settings   Full path to settings file (user, password)
               output   Name for output directory where to store downloaded Sentinel data
           footprints   Only supported for download from ESA_Copernicus Open Access Hub
                  map   Name of input vector map to define Area of Interest (AOI)
        area_relation   ESA Copernicus Open Access Hub allows all three, USGS Earth Explorer only 'Intersects' option
                        options: Intersects,Contains,IsWithin
                        default: Intersects
               clouds   Maximum cloud cover percentage for Sentinel scene
          producttype   USGS Earth Explorer only supports S2MSI1C
                        options: SLC,GRD,OCN,S2MSI1C,S2MSI2A,S2MSI2Ap,
                                 S3OL1EFR,S3OL1ERR,S3OL1SPC,S3OL1RAC,
                                 S3OL2WFR,S3OL2WRR,S3OL2LFR,S3OL2LRR,
                                 S3SL2LST,S3SL2FRP,S3SY2SYN,S3SY2VGP,
                                 S3SY2VG1,S3SY2V10,S3SY2AOD,S3SR2LAN
                        default: S2MSI2A
                start   Start date ('YYYY-MM-DD')
                  end   End date ('YYYY-MM-DD')
                limit   Limit number of Sentinel products
                query   USGS Earth Explorer only supports query options "identifier", "filename" (in ESA name format) or "usgs_identifier" (in USGS name format)
                 uuid   List of UUID to download
  relativeorbitnumber   _Only supported by ESA Copernicus Open Access Hub.
                sleep   Sleep time in minutes before retrying to download data from ESA LTA
                retry   Maximum number of retries before skipping to the next scene at ESA LTA
                        default: 5
           datasource   Default is ESA Copernicus Open Access Hub (ESA_COAH), but Sentinel-2 L1C data can also be acquired from USGS Earth Explorer (USGS_EE) or Google Cloud Storage (GCS)
                        options: ESA_COAH,USGS_EE,GCS
                        default: ESA_COAH
                 sort   Sort by values in given order
                        options: ingestiondate,cloudcoverpercentage,footprint
                        default: cloudcoverpercentage,ingestiondate,footprint
                order   Sort order (see sort parameter)
                        options: asc,desc
                        default: asc

List available extensions in the official GRASS GIS Addons repository:

C:\Users\User>g.extension -l
List of available extensions (modules):
d.explanation.plot
d.frame
d.mon2
d.region.grid
d.vect.colbp
d.vect.colhist
d.vect.thematic2
db.join
g.citation
g.compare.md5
g.copyall
g.download.location
g.isis3mt
g.proj.all
g.proj.identify
g.projpicker
g.rename.many
i.ann.maskrcnn
...

@petrasovaa
Copy link
Contributor

Thanks for looking into it. It looks like with the changes in your PR get_version_branch gets called in a single place (when running g.extension -l), no? While I believe this works, I find this to be overly complicated, especially when the parsing can stop working any time when Github decides to change the page. My suggestion is to simply change the code of get_version_branch to return the current major version for Windows.

It seems you disabled the post processing on Windows, so I was wondering what exactly is the consequence of that?

@tmszi
Copy link
Member Author

tmszi commented Sep 21, 2023

Thanks for looking into it. It looks like with the changes in your PR get_version_branch gets called in a single place (when running g.extension -l), no? While I believe this works, I find this to be overly complicated, especially when the parsing can stop working any time when Github decides to change the page. My suggestion is to simply change the code of get_version_branch to return the current major version for Windows.

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 get_version_branch() function (line 524) :

if version_branch not in gs.decode(branch):
version_branch = "grass{}".format(int(major_version) - 1)

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).

It seems you disabled the post processing on Windows, so I was wondering what exactly is the consequence of that?

Postprocessing addons man HTML page is done during compilation process on the server side.

@wenzeslaus
Copy link
Member

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).

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).

@tmszi
Copy link
Member Author

tmszi commented Sep 24, 2023

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.

C:\Users\User>g.extension -l
ERROR: Getting official addons repository branches from
       <https://api.github.com/repos/OSGeo/grass-addons/branches> failed.
       The server couldn't fulfill the request and return status code <403>
       <Request forbidden -- authorization will not help>. GitHub REST API
       rate limit was exceeded 60 requests per hour per IP address. Try
       list official addons at <Sunday Sep 24 06:31:17 2023> again, please.

Copy link
Contributor

@veroandreo veroandreo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor wording fixes

scripts/g.extension/g.extension.py Outdated Show resolved Hide resolved
scripts/g.extension/g.extension.py Outdated Show resolved Hide resolved
scripts/g.extension/g.extension.py Outdated Show resolved Hide resolved
@petrasovaa
Copy link
Contributor

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 get_version_branch is used in different contexts and for Windows the behavior doesn't necessarily make much sense. I want to be careful to add too much complex code where we don't need it.

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.

@tmszi
Copy link
Member Author

tmszi commented Sep 26, 2023

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.

Currently, this PR only uses the GitHub REST API in one place in the get_version_branch function and only on MS Windows and only to list g.extension -l extensions.

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.

I think part of the problem is that the code in general could be better structured, the get_version_branch is used in different contexts and for Windows the behavior doesn't necessarily make much sense. I want to be careful to add too much complex code where we don't need it.

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 get_version_branch() function (line 524) :

if version_branch not in gs.decode(branch):
version_branch = "grass{}".format(int(major_version) - 1)

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 g.extension -l stops working (as it was in case of transition from major version from 7 to version 8).

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.

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.
@tmszi
Copy link
Member Author

tmszi commented Sep 26, 2023

I incorporated a4ceb4c your suggestion that simply return the major version branch.

@tmszi tmszi modified the milestones: 8.4.0, 8.3.1 Sep 26, 2023
Copy link
Contributor

@petrasovaa petrasovaa left a 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.

@tmszi tmszi merged commit 5deae3a into OSGeo:main Sep 27, 2023
18 checks passed
tmszi added a commit to tmszi/grass that referenced this pull request Sep 27, 2023
@tmszi tmszi removed the backport to 8.3 PR needs to be backported to release branch 8.3 label Sep 27, 2023
@tmszi tmszi deleted the g_extension-fix-install-addon-on-win32-os-without-git branch September 27, 2023 10:46
landam pushed a commit to landam/grass that referenced this pull request Oct 25, 2023
@hellik
Copy link
Member

hellik commented Oct 26, 2023

tested by

System Info                                                                     
GRASS version: 8.3.1                                                            
Code revision: exported                                                         
Build date: 2023-10-25                                                          
Build platform: x86_64-w64-mingw32                                              
GDAL: 3.7.2                                                                     
PROJ: 9.3.0                                                                     
GEOS: 3.12.0                                                                    
SQLite: 3.41.1                                                                  
Python: 3.9.5                                                                   
wxPython: 4.2.0                                                                 
Platform: Windows-10-10.0.19045-SP0 (OSGeo4W) 
C:\Users\myuser\Documents>g.extension -l
List of available extensions (modules):
ERROR: Download file from
       <https://grass.osgeo.org/addons/grass8/modules.xml>, failed. File is
       not on the server or check your internet connection.

from the GUI

Unable to load extensions. Download file from
<https://grass.osgeo.org/addons/grass8/modules.xml>, failed.
File is not on the server or check your internet connection.

Exception in thread
Thread-14
:
Traceback (most recent call last):
  File "C:\OSGeo4W\apps\Python39\lib\threading.py", line
954, in _bootstrap_inner

self.run()
  File "C:\OSGeo4W\apps\grass\grass83\gui\wxpython\core\gthr
ead.py", line 154, in __run

self.__run_backup()
  File "C:\OSGeo4W\apps\grass\grass83\gui\wxpython\core\gthr
ead.py", line 117, in run

ret = vars()["callable"](*args, **kwds)
  File "C:\OSGeo4W\apps\grass\grass83\gui\wxpython\modules\e
xtensions.py", line 369, in Load

raise GException(_("Unable to load extensions. %s") % msg)
core.gcmd
.
GException
:
Unable to load extensions. Download file from
<https://grass.osgeo.org/addons/grass8/modules.xml>, failed.
File is not on the server or check your internet connection.

the xml file is on the server:

[DIR]	logs/	2023-10-26 07:12 	- 	 
[ ]	modules.xml	2023-10-26 07:14 	191K	 

is this fix meant for 8.4?

@tmszi
Copy link
Member Author

tmszi commented Oct 26, 2023

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.

@hellik
Copy link
Member

hellik commented Oct 26, 2023

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.

@hellik
Copy link
Member

hellik commented Oct 26, 2023

and the error does not appear on a very MS Windows installation.

how can we support users on not such recent windows boxes?

neteler pushed a commit to nilason/grass that referenced this pull request Nov 7, 2023
HuidaeCho pushed a commit to HuidaeCho/grass that referenced this pull request Jan 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Python Related code is in Python Windows Microsoft Windows specific
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] g.extension: installation addon fails because Git program is missing
5 participants