temporal: handle SQLite < 3.33 #3723

ninsbl · 2024-05-19T20:10:42Z

This is an attempt to address:
#3466

Not sure how to test with Ubuntu 20.04. It is not part of CI, and I do not have Ubuntu 20.04 at hand right now...

I would appreciate if anyone with Ubuntu 20.04 could run temporal tests with this PR applied...

echoix · 2024-05-19T20:54:18Z

Travis still runs Ubuntu focal. We could create a second PR with temporary changes to the config to run these tests, and never merge that PR.

echoix · 2024-05-20T00:37:45Z

I'm running a modified CI run based on your PR. The current Ubuntu workflow built successfully, now waiting for the tests to finish in about 30 min.

https://github.com/echoix/grass/actions/runs/9151666333/job/25158041723?pr=91

It has SQLite 3.31.1


Python 3.8.10
Python 3.8.10
git version 2.45.1
GRASS GIS 8.4.0dev
Geographic Resources Analysis Support System (GRASS) is Copyright,
1999-2024 by the GRASS Development Team, and licensed under terms of the
GNU General Public License (GPL) version >=2.

This GRASS GIS 8.4.0dev release is coordinated and produced by
the GRASS Development Team with contributions from all over the world.

This program is distributed in the hope that it will be useful, but
WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
General Public License for more details.
Option --tmp-location is deprecated, use --tmp-project instead
GRASS 8.4.0dev (2024)
PROJ: 6.3.1
GDAL/OGR: 3.0.4
GEOS: 3.8.0
SQLite: 3.31.1
Option --tmp-location is deprecated, use --tmp-project instead
Python: 3.8.10 (default, Nov 22 2023, 10:22:35) [GCC 9.4.0]

wenzeslaus

Thank you. This solves it!

I tested locally with Ubuntu 20.04 and all pytest tests run (at least one temporal one was failing previously).

Please, include in the commit message more details on when to delete what parts when the time comes. I guess reverting this commit will do the trick, but it is not clear to me what is legacy from reading the code and seeing the _v3 and _old files.

echoix

As mentioned, indications on what and when to clean up later would be helpful.

python/grass/temporal/abstract_space_time_dataset.py

echoix · 2024-05-20T00:46:48Z

@wenzeslaus great for pytest, as my CI run was only for the gunittest-based tests. So together we have complete coverage

wenzeslaus · 2024-05-20T00:50:52Z

...create a second PR with temporary changes to the config to run these tests...

The "old" way of original reporter (myself) testing it was just simple in this case. ;-)

Generally, I think the idea would be to test the full supported range of dependencies in the CI. We sort of do that with Python and C in couple places. I think the same idea was there for Ubuntu to get old version of dependencies determined common Linux distro. I'm not sure why we dropped 20.04 in GitHub Actions. Did GitHub make that deprecated? I don't see any warnings in your run, @echoix. (?)

echoix · 2024-05-20T01:06:18Z

I'm not sure I was following the CI when Ubuntu 20.04 was dropped (if it was ever there). I know the transition that dropped things like centos and others were made before too.

One consideration that we need to have, is that the CI runs we have are already really long, and multiple. We are sometimes fighting for CI time against other OSGeo projects, so adding 1 or 2 hours per PR commit + per PR merge should be thought of a little bit.

Another one, is that now the ubuntu-24.04 runner images are out (in beta, and the proportion of machines with that image is being scaled, the capacity will be balanced in the next weeks). If we were to add some platforms, I would run with 24.04 instead.
GitHub's actions/runner-images also mention that: "GA images are eventually deprecated according to our guidelines as we only support the latest 2 versions of an OS." https://github.com/actions/runner-images?tab=readme-ov-file#ga
So 20.04 is way closer to the end.

If you aren't paying by minute, maybe for now something reasonable would be to make Travis run gunittest tests while keeping it at Ubuntu 20.04 (focal), as it is compiling but doing nothing based on it, and we aren't fighting over capacity for it now.

echoix · 2024-05-20T01:14:47Z

I'm running a modified CI run based on your PR. The current Ubuntu workflow built successfully, now waiting for the tests to finish in about 30 min.

https://github.com/echoix/grass/actions/runs/9151666333/job/25158041723?pr=91

It has SQLite 3.31.1


Python 3.8.10
Python 3.8.10
git version 2.45.1
GRASS GIS 8.4.0dev
Geographic Resources Analysis Support System (GRASS) is Copyright,
1999-2024 by the GRASS Development Team, and licensed under terms of the
GNU General Public License (GPL) version >=2.

This GRASS GIS 8.4.0dev release is coordinated and produced by
the GRASS Development Team with contributions from all over the world.

This program is distributed in the hope that it will be useful, but
WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
General Public License for more details.
Option --tmp-location is deprecated, use --tmp-project instead
GRASS 8.4.0dev (2024)
PROJ: 6.3.1
GDAL/OGR: 3.0.4
GEOS: 3.8.0
SQLite: 3.31.1
Option --tmp-location is deprecated, use --tmp-project instead
Python: 3.8.10 (default, Nov 22 2023, 10:22:35) [GCC 9.4.0]

There were test failures for the temporal tests in gunittest (2 files, 2 failed tests). But there was also a failure in the gunittest in folders other than temporal (1 file, 1 failed test)

https://github.com/echoix/grass/actions/runs/9151666333/job/25158041723?pr=91


Running ./temporal/t.register/testsuite/test_t_register_raster_different_local.py...
Running ./temporal/t.register/testsuite/test_t_register_raster_file.py...
========================================================================
Default TGIS driver / database set to:
driver: sqlite
database: $GISDBASE/$LOCATION_NAME/$MAPSET/tgis/sqlite.db
WARNING: Temporal database connection defined as:
/home/runner/nc_spm_full_v2alpha2/__temporal_t_register_test_t_register_raster_file_fv_az1154_465_34326/tgis/sqlite.db
But database file does not exist.
Creating temporal database: /home/runner/nc_spm_full_v2alpha2/__temporal_t_register_test_t_register_raster_file_fv_az1154_465_34326/tgis/sqlite.db
...F...
======================================================================
FAIL: test_with_mapset_and_semantic_label (__main__.TestRegisterFile)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "temporal/t.register/testsuite/test_t_register_raster_file.py", line 355, in test_with_mapset_and_semantic_label
    self.assertEqual(strds.metadata.get_number_of_semantic_labels(), 1)
AssertionError: None != 1
----------------------------------------------------------------------
Ran 7 tests in 24.325s
FAILED (failures=1)
========================================================================
FAILED ./temporal/t.register/testsuite/test_t_register_raster_file.py (1 test failed)
Running ./temporal/t.register/testsuite/test_t_register_raster_mapmetadata.py...
========================================================================
Default TGIS driver / database set to:
driver: sqlite
database: $GISDBASE/$LOCATION_NAME/$MAPSET/tgis/sqlite.db
WARNING: Temporal database connection defined as:
/home/runner/nc_spm_full_v2alpha2/__temporal_t_register_test_t_register_raster_mapmetadata_fv_az1154_465_34326/tgis/sqlite.db
But database file does not exist.
Creating temporal database: /home/runner/nc_spm_full_v2alpha2/__temporal_t_register_test_t_register_raster_mapmetadata_fv_az1154_465_34326/tgis/sqlite.db
F
======================================================================
FAIL: test_register_metadata_from_file (__main__.TestRegisterMapMetadata)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "temporal/t.register/testsuite/test_t_register_raster_mapmetadata.py", line 116, in test_register_metadata_from_file
    self.assertEqual(strds.metadata.get_number_of_semantic_labels(), 1)
AssertionError: None != 1
----------------------------------------------------------------------
Ran 1 test in 6.221s
FAILED (failures=1)
========================================================================
FAILED ./temporal/t.register/testsuite/test_t_register_raster_mapmetadata.py (1 test failed)
Running ./temporal/t.rename/testsuite/test_t_rename.py...
Running ./temporal/t.sample/testsuite/test.t.sample.sh...
Running ./temporal/t.shift/testsuite/test_shift.py...

And https://github.com/echoix/grass/actions/runs/9151666333/job/25158041637


Running ./scripts/db.univar/testsuite/test_db_univar.py...
Running ./scripts/g.extension/testsuite/doctest.sh...
Running ./scripts/g.extension/testsuite/test_addons_download.py...
========================================================================
WARNING: Bug in UI description. Missing module description
.F.......
======================================================================
FAIL: test_github_download_official_module_src_code_only (__main__.TestModuleDownloadFromDifferentSources)
Test download extension source code only from official addons
----------------------------------------------------------------------
Traceback (most recent call last):
  File "scripts/g.extension/testsuite/test_addons_download.py", line 190, in test_github_download_official_module_src_code_only
    self.assertTrue(ext_path.exists())
AssertionError: False is not true

----------------------------------------------------------------------
Ran 9 tests in 19.462s
FAILED (failures=1)
========================================================================
FAILED ./scripts/g.extension/testsuite/test_addons_download.py (1 test failed)
Running ./scripts/g.extension/testsuite/test_addons_modules.py...
Running ./scripts/g.extension/testsuite/test_addons_toolboxes.py...
Running ./scripts/i.band.library/testsuite/test_i_band_library.py...

ninsbl · 2024-05-20T19:43:37Z

Thanks for the effort with testing. I simulated the test here by locally and temporarily inverting the check for the SQLite version. And now tests with SQLite < 3.33 should pass... Please try again!

echoix · 2024-05-20T20:11:14Z

Launched https://github.com/echoix/grass/actions/runs/9164388688/job/25195578677?pr=91 and https://github.com/echoix/grass/actions/runs/9164388688/job/25195578465?pr=91

echoix · 2024-05-20T20:47:25Z

Launched https://github.com/echoix/grass/actions/runs/9164388688/job/25195578677?pr=91 and https://github.com/echoix/grass/actions/runs/9164388688/job/25195578465?pr=91

For the non-temporal:

Running ./scripts/g.extension/testsuite/doctest.sh...
Running ./scripts/g.extension/testsuite/test_addons_download.py...
========================================================================
WARNING: Bug in UI description. Missing module description
.F.......
======================================================================
FAIL: test_github_download_official_module_src_code_only (__main__.TestModuleDownloadFromDifferentSources)
Test download extension source code only from official addons
----------------------------------------------------------------------
Traceback (most recent call last):
  File "scripts/g.extension/testsuite/test_addons_download.py", line 190, in test_github_download_official_module_src_code_only
    self.assertTrue(ext_path.exists())
AssertionError: False is not true
----------------------------------------------------------------------
Ran 9 tests in 19.511s
FAILED (failures=1)
========================================================================
FAILED ./scripts/g.extension/testsuite/test_addons_download.py (1 test failed)
Running ./scripts/g.extension/testsuite/test_addons_modules.py...
Running ./scripts/g.extension/testsuite/test_addons_toolboxes.py...

no errors reported in the temporal folder

wenzeslaus · 2024-05-21T00:32:54Z

I don't get it. All pytest tests run for me, but I tried scripts/g.extension/testsuite/test_addons_download.py from the gunittest test suite to understand what @echoix is getting and I got:

Fetching <db.join> from GRASS GIS Addons repository (be patient)...
svn: E170013: Unable to connect to a repository at URL 'https://github.com/OSGeo/grass-addons/branches/grass8/src/db/db.join'
svn: E160013: '/OSGeo/grass-addons/branches/grass8/src/db/db.join' path not found
ERROR: GRASS Addons <db.join> not found

Clearly unrelated to this PR. I rebuilt GRASS and this branch has a recent merge from main, so it has #2895. (?)

ninsbl · 2024-05-22T20:26:52Z

As the AddOn issue is unrelated, do you feel this is ready to merge?

echoix · 2024-05-22T23:00:11Z

I don't have any problems with merging this

ninsbl added 3 commits May 19, 2024 21:49

handle SQLite < 3.33

a0a643d

handle SQLite < 3.33

7eaf464

handle SQLite < 3.33

9a0997d

ninsbl added temporal Related to temporal data processing Python Related code is in Python database Related to database management libraries labels May 19, 2024

ninsbl added this to the 8.4.0 milestone May 19, 2024

ninsbl requested review from wenzeslaus and echoix May 19, 2024 20:10

ninsbl mentioned this pull request May 19, 2024

[Bug] Updating STRDS metadata seems unnecessarily slow #3136

Closed

Merge branch 'main' into tgis_old_sqlite

bb08994

wenzeslaus previously approved these changes May 20, 2024

View reviewed changes

echoix reviewed May 20, 2024

View reviewed changes

python/grass/temporal/abstract_space_time_dataset.py Show resolved Hide resolved

fix semantic_label update

4f0137d

ninsbl dismissed wenzeslaus’s stale review via 4f0137d May 20, 2024 19:38

echoix approved these changes May 22, 2024

View reviewed changes

echoix merged commit 132755b into OSGeo:main May 22, 2024
26 checks passed

This was referenced May 24, 2024

[Feat] temporal: revert #3723 (fix for SQLite version < 3.33) #3736

Open

[Bug] temporal tools fail with SQLite 3.31 #3466

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

temporal: handle SQLite < 3.33 #3723

temporal: handle SQLite < 3.33 #3723

ninsbl commented May 19, 2024

echoix commented May 19, 2024

echoix commented May 20, 2024 •

edited

Loading

wenzeslaus left a comment

echoix left a comment

echoix commented May 20, 2024

wenzeslaus commented May 20, 2024

echoix commented May 20, 2024

echoix commented May 20, 2024

ninsbl commented May 20, 2024

echoix commented May 20, 2024

echoix commented May 20, 2024

wenzeslaus commented May 21, 2024

ninsbl commented May 22, 2024

echoix commented May 22, 2024

temporal: handle SQLite < 3.33 #3723

temporal: handle SQLite < 3.33 #3723

Conversation

ninsbl commented May 19, 2024

echoix commented May 19, 2024

echoix commented May 20, 2024 • edited Loading

wenzeslaus left a comment

Choose a reason for hiding this comment

echoix left a comment

Choose a reason for hiding this comment

echoix commented May 20, 2024

wenzeslaus commented May 20, 2024

echoix commented May 20, 2024

echoix commented May 20, 2024

ninsbl commented May 20, 2024

echoix commented May 20, 2024

echoix commented May 20, 2024

wenzeslaus commented May 21, 2024

ninsbl commented May 22, 2024

echoix commented May 22, 2024

echoix commented May 20, 2024 •

edited

Loading