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

Safety dependencies repeatedly break our project #511

Open
washeck opened this issue Apr 3, 2024 · 5 comments · May be fixed by #517 or #518
Open

Safety dependencies repeatedly break our project #511

washeck opened this issue Apr 3, 2024 · 5 comments · May be fixed by #517 or #518

Comments

@washeck
Copy link

washeck commented Apr 3, 2024

@Jwomers

I want to express my strong disappointment with the way safety is developed.

For us it is just a support tool, one of the many small tools such as linters, code formatters and other nice but not critical tools meant to make our product better and safe developers time.

We started to use safetycli because we are migrating from pipenv to poetry and we cannot use pipenv check anymore. We are not interested in any other features, we just want the tool to work same as pipenv check.

Since the migration to safety, I am seeing many complications I have not encountered before. There was #495 , #447 , fixes were available only in beta version

Today I ran pipenv lock just to upgrade vulnerable pillow library and I see that some typer appeared as a new dependency and it does not work

Traceback (most recent call last):
  File "<our_venv>/bin/safety", line 5, in <module>
    from safety.cli import cli
  File "<our_venv>/lib/python3.11/site-packages/safety/cli.py", line 19, in <module>
    from safety import safety
  File "<our_venv>/lib/python3.11/site-packages/safety/safety.py", line 34, in <module>
    from .models import Vulnerability, CVE, Severity, Fix, is_pinned_requirement, SafetyRequirement
  File "<our_venv>/lib/python3.11/site-packages/safety/models.py", line 300, in <module>
    from safety.auth.models import Auth
  File "<our_venv>/lib/python3.11/site-packages/safety/auth/__init__.py", line 3, in <module>
    from .cli import auth
  File "<our_venv>/lib/python3.11/site-packages/safety/auth/cli.py", line 17, in <module>
    from typer import Typer
ImportError: cannot import name 'Typer' from 'typer' (unknown location)

I don't want to spend my time on your experiments with typing. While typing seems like interesting addition to Python, please keep pydantic, mypy, typer or whatever typing technology you find interesting in you dev dependencies.

@yeisonvargasf
Copy link
Member

Hey there, @washeck.

I'm sorry to hear about your issue with some dependencies of Safety. I agree that things like that should work smoother and not disturb you as a user as much as happened here. But let me quickly explain why some of those dependencies exist. Mainly to be transparent and prevent any confusion.

The typer we are using is https://typer.tiangolo.com/, used to build CLI apps, not as a dev dependency. To remove any misunderstanding here. We are switching to Typer because it is easier to use and reduces the code we have to write for validation on options, etc. It is included as a dependency in our setup.cfg file going forward.

Others like packaging and pydantic are needed not just for us as developers but also for some functionality in our product.

As we are a small growing company/product, it might happen that we run into migration or deprecation issues. Something that isn't great and that doesn't feel good to the user, but some of it has to happen, unfortunately. We are trying to be better about this.

I hope this relieves some of your concerns with our dependencies.

Related to the issue, I recommend installing Safety in an isolated environment and using the scan command to target your project root; with that approach, you won't have dependency issues. If you require further help, please let me know the safety and Python versions, plus any other details to help identify if this is a Safety bug.

@washeck
Copy link
Author

washeck commented Apr 4, 2024

I think the issue we ran into is tiangolo/typer#790 - typer probably issued somehow broken release.

Based on you advice to use safety in isolated environment, I was thinking about running it in docker but it seems like either the doc https://docs.safetycli.com/safety-docs/installation/docker-containers is outdated or the pyupio/safety is not maintained. Running the command from doc

$ docker run --rm -ti pyupio/safety --version
docker: Error response from daemon: failed to create task for container: failed to create shim task: OCI runtime create failed: runc create failed: unable to start container process: exec: "--version": executable file not found in $PATH: unknown.

as thee is probably wrong entrypoint in the docker file. Using the working command

$ docker run --rm -ti pyupio/safety safety --version
safety, version 1.10.3

shows the image is outdated.

Do you provide up-to-date one?

@yacine-harbi
Copy link

To fix the issue, you have to uninstall and reinstall typer ... I know (source).

@yeisonvargasf
Copy link
Member

@washeck, nice catch with the docs there. The team updated the docs: https://docs.safetycli.com/safety-docs/installation/docker-containers

We moved our docker images to ghcr.io, docker hub is there only for legacy support. You can use ghcr.io/pyupio/safety:3.1 if you want to pin the docker image.

@andy-maier
Copy link

Right now, safety does not require any specific version of typer: https://github.com/pyupio/safety/blob/main/setup.cfg#L51C5-L51C10

As the issue was solved in typer 0.12.1, safety 3.x should increase its minimum version of typer to >=0.12.1.

jeff-at-tamer-dot-codes added a commit to jeff-at-tamer-dot-codes/safety that referenced this issue Apr 29, 2024
Safety CLI depends on _underscore methods defined in typer which are not guaranteed to be stable, even across bugfix version changes. Locking typer to version 0.12.3, while inconvenient, is unfortunately necessary to ensure that refactorings in typer do not negatively impact our users. Helps fix pyupio#511
jeff-at-tamer-dot-codes added a commit to jeff-at-tamer-dot-codes/safety that referenced this issue Apr 29, 2024
jeff-at-tamer-dot-codes added a commit to jeff-at-tamer-dot-codes/safety that referenced this issue Apr 29, 2024
…sible.

Note: this commit is NOT a noop. Switching to ~= will prevent unintentionally breaking our users due to a major version change out of our control upstream. For packages still in beta (i.e. packages whose major versions equal zero), we use the latest "major.minor.bugfix" as the version identifier; otherwise we use "major.minor". Tested manually. I believe this PR fixes pyupio#511 in spirit.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment