recast-hep / recast-atlas

CLI for ATLAS RECAST contributors
https://recast.docs.cern.ch/
Apache License 2.0
7 stars 5 forks source link

require 'jsonschema<4.0' #74

Closed lukasheinrich closed 2 years ago

lukasheinrich commented 3 years ago

this seems to be required by reana

matthewfeickert commented 3 years ago

If so, then it isn't listed in the reana-client requirements but is listed in reana-commons requirements

install_requires = [
    "bravado>=10.2,<10.4",
    "checksumdir>=1.1.4,<1.2",
    "click>=7.0",
    "fs>=2.0",
    "jsonschema[format]>=3.0.1,<4.0.0",
    "kombu>=4.6,<4.7",
    "mock>=3.0,<4",
    "PyYAML>=5.1,<6.0",
    "Werkzeug>=0.14.1",
]

If a user installs with the [reana] extra, then this is already specified for them so I don't think we need to place this requirement on recast-atlas as there's nothing in its API that explicitly needs this.

$ docker run --rm -it python:3.8 /bin/bash
root@989de5cf926b:/# python -m venv venv && . venv/bin/activate
(venv) root@989de5cf926b:/# python -m pip install --upgrade pip "setuptools<58.0.0" wheel
(venv) root@989de5cf926b:/# python -m pip install recast-atlas[develop,local,kubernetes,reana]
(venv) root@989de5cf926b:/# pip show jsonschema
Name: jsonschema
Version: 3.2.0
Summary: An implementation of JSON Schema validation for Python
Home-page: https://github.com/Julian/jsonschema
Author: Julian Berman
Author-email: Julian@GrayVines.com
License: UNKNOWN
Location: /venv/lib/python3.8/site-packages
Requires: attrs, pyrsistent, setuptools, six
Required-by: bravado-core, reana-commons, recast-atlas, swagger-spec-validator, yadage-schemas

Do you agree?

matthewfeickert commented 3 years ago

As reana-commons has it in the install_requires, this will be handled correctly even if the user installs the reana extra later:

$ docker run --rm -it python:3.8 /bin/bash
root@16e086370a2c:/# python -m venv venv && . venv/bin/activate
(venv) root@16e086370a2c:/# python -m pip --quiet install --upgrade pip "setuptools<58.0.0" wheel
(venv) root@16e086370a2c:/# python -m pip install recast-atlas
(venv) root@16e086370a2c:/# pip freeze | grep jsonschema
jsonschema==4.1.2
(venv) root@16e086370a2c:/# python -m pip install recast-atlas[reana]
(venv) root@16e086370a2c:/# pip freeze | grep jsonschema
jsonschema==3.2.0

So I think everything is working fine and as expected. @lukasheinrich do you have an example of this failing?

matthewfeickert commented 2 years ago

Closing this as I think this is resolved. Please reopen if not.