fix tests to allow for variable STAC versions in output #73
Conversation
There was a problem hiding this comment.
Without some kind of dependapot checking packages regularly, I'm expecting maintenance creep and vulnerabilities in the long run.
For example, requests and lxml are regularly updated to resolve security issues. I somewhat like the fact that those are auto-bumped to latest version. The "pinned" versions, if needed, are available in the docker.
Note also that limiting MAJOR is not really a fix, just as in the case of pystac. It is a seemingly "MINOR" update that broke the tests by changing STAC version entirely.
tests/test_standalone_stac_item.py
Outdated
| @@ -86,5 +88,6 @@ def test_cmip6_stac_thredds_catalog_parsing(): | |||
| ref_file = os.path.join(CUR_DIR, "data/stac_collection_testdata_xclim_cmip6_catalog.json") | |||
| with open(ref_file, mode="r", encoding="utf-8") as ff: | |||
| reference = json.load(ff) | |||
| reference["stac_version"] = get_stac_version() | |||
There was a problem hiding this comment.
Seems problematic IMO.
There are different properties (eg: new bands) in STAC 1.1 for which stac_version should not be blindly "overridden", but actually migrated.
If there was an error when using pystac within stac-populator, I'd argue this is probably a mishandling somewhere which should be addressed. I find this test properly early-detected an issue that must be investigated further, since pystac is supposed to support both versions, and do appropriate migrations as needed.
Are we not simply ignoring a potential issue if the catalog targeted by stac-populator still uses STAC 1.0? We cannot just push a STAC 1.1 to it, and the populator code should adapt to this (if needed).
There was a problem hiding this comment.
See https://github.com/stac-utils/pystac/blob/main/CHANGELOG.md#v1121
Maybe we should have migrate=False explicitly?
There was a problem hiding this comment.
Are we not simply ignoring a potential issue if the catalog targeted by stac-populator still uses STAC 1.0? We cannot just push a STAC 1.1 to it, and the populator code should adapt to this (if needed).
I don't think that is what this is testing. This does not try to upload any data to an existing STAC server so it doesn't have any information from a server to know what version it is running.
We should definitely add a test that checks for that but that should be additional to this one.
There are different properties (eg: new bands) in STAC 1.1 for which stac_version should not be blindly "overridden", but actually migrated.
I'll update this test to ensure that any migrations beyond just the "stac_version" between versions are being tested as well.
Ok let's add a dependabot
We're not just developing a docker image though, this is also usable as a package and dependency versions should be managed properly so they don't break things for other users.
Ok so we'll keep pystac more limited. Good to know. |
FYI I don't have sufficient permissions to enable dependabot for this repo. If you have permissions @huard or @fmigneault please feel free to set it up. |
Tests currently assume that the STAC version in item and collection JSONs will always be
1.0.0.Since we don't pin a specific version of
pystac, a new version ofpystacuses STAC version1.1.0which now causes tests to fail.This PR updates tests to allow for item and collection JSONs to have a
stac_versionfield that matches the version used by the installed version ofpystac.As part of this PR, I also updated the dependency versions to not allow major version upgrades automatically. For example,
pystacis now pinned to version~=1.12which will mean is will not install version 2+. This should avoid similar breaking changes caused by dependency updates in the future.