Skip to content

Allow non-types dependencies #5768

Closed
typeshed-internal/stub_uploader
#61

Description

@srittau

This was previously discussed in #5618 and came up in #5765. I think the consensus is that we should allow this. This issue is mainly to track this.

Activity

  1. intgr commented on Aug 5, 2021

    @intgr
    Contributor

    Copying relevant discussion from #5618 for context:

    srittau commented on Jun 18

    Not speaking about stubs packages in general, but I think it's reasonable if typeshed packages depend on dependencies of the corresponding upstream package. I.e. if package foo depends on cryptography, it's fine that our foo-types depends on cryptography as well, since it doesn't introduce "new" dependencies. Apart from that we should vet dependencies carefully.

    JukkaL commented on Jun 18

    I agree with srittau.

    I think that it would be sufficient to maintain an allowlist of vetted dependencies. I'd expect that the number of such dependencies will be fairly small, so it shouldn't be much effort to maintain.

  2. hmc-cs-mdrissi commented on Feb 20, 2022

    @hmc-cs-mdrissi
    Contributor

    I'm running into this issue for tensorflow stubs. Most tensorflow type annotations depend on a couple basic numpy types. So at moment my options look like don't include numpy types and have most annotations be too strict or have Any almost everywhere.

    What's needed to work on this task? I can see one issue is pyright check produces errors from dependency not being installed. Is the prerequisite being able to run checks for only changed stubs as mentioned here, #5952

  3. Akuli commented on Sep 7, 2022

    @Akuli
    Collaborator

    If I understand correctly, the next steps are:

    • Add a check to tests/check_consistent.py that fails if stubs/foo depends on anything else than:
      • A types-bar package where bar is so that stubs/bar exists in typeshed
      • Or a dependency of foo on pypi that has py.typed
        • We can use pypi's API for this, although checking the presence of py.typed with just the API is tricky. Can we just assume that py.typed exists, or do we need to potentially download the sdist to check? How about packages that don't provide sdist?
    • Change one dependency of one stubs-package to use upstream types
    • Make a pull request
    • Fix pull request until CI is green
    • Before merging, run the third-party stubs check in daily.yml passes (this can be ran manually)

    A potential problem is if packages foo and bar have dependencies that cannot be installed simultaneously. Then daily.yml will not work at all. I think we should fix this later, in a separate issue and a separate PR, because users are already complaining about not being able to use upstream type hints, and this issue won't come up until we have many non-stubs dependencies in typeshed.

    If we have a package foo with dependency bar and stubs in stubs/foo, we probably want types-foo to depend on bar and not a specific version like bar==1.2.3, so that you can upgrade to bar==1.2.4 without breaking your pip install -r requirements.txt -r requirements-dev.txt (assuming bar==1.2.4 is in requirements.txt and types-foo==x.y.z is in requirements-dev.txt).

    Is this all there's to it, or am I missing something? I haven't followed this discussion in detail before, because it seemed to be complicated and also spread up across many places.

  4. hauntsaninja commented on Sep 7, 2022

    @hauntsaninja
    Collaborator

    We need changes to mypy_test and the others, see e.g. #5952 (comment)

    We need some changes in stub_uploader as well, which has validation around this. And I think it's a good opportunity to do #8312

    I have the stub_uploader patch locally, but haven't PR-ed it because of some conflicts with typeshed-internal/stub_uploader#57 which I was hoping would get merged

  5. Akuli commented on Sep 7, 2022

    @Akuli
    Collaborator

    I'm mostly confused about "changes to mypy_test and the others" part.

    • Why exactly is Testing third-party stubs in isolated environments #5952 (comment) relevant now? It says "I personally wouldn't hate getting started on [this issue] with a global venv" which is what I'm suggesting.
    • Is all this included in my "Fix pull request until CI is green" part, or is there something subtle that our CI wouldn't catch if forgotten?
  6. hauntsaninja commented on Sep 7, 2022

    @hauntsaninja
    Collaborator

    Our CI wouldn't catch changes needed for stub_uploader. But yes, everything else is probably covered under "get CI green".

    Here's code that checks if a package contains a py.typed:

    async def package_contains_py_typed(release_to_download: dict[str, Any], session: aiohttp.ClientSession) -> bool:

    But we probably don't need it; we should get errors in type checking tests if something doesn't have py.typed. People have previously suggested using an allowlist of packages (I believe for security reasons), which would also take care of that.

  7. Akuli commented on Sep 7, 2022

    @Akuli
    Collaborator

    I personally don't see how duplicating the dependencies into a separate allowlist in the same repository improves security.

  8. hauntsaninja commented on Sep 7, 2022

    @hauntsaninja
    Collaborator

    It'd be in stub_uploader. I don't have opinions on it, but mentioned it as a thing that has been previously discussed, since e.g. an allowlist would make validation like checking for py.typed not very important.

  9. ilevkivskyi commented on Sep 19, 2022

    @ilevkivskyi
    Member

    As I mentioned on the PR typeshed-internal/stub_uploader#59 this introduces a security hole. Having an allowlist in stub_uploader (to which intentionally only few maintainers have access) is a possible mitigation, but I want to first hear what other people (in particular @JukkaL and @JelleZijlstra) think about this.

  10. AlexWaygood commented on Sep 19, 2022

    @AlexWaygood
    Member

    this introduces a security hole

    Could you expand on what the security risk is, exactly? Like @Akuli, I'm not sure I've ever seen it fully explained (probably because I'm somewhat new as a maintainer :-)

    I agree with the consensus on this issue that this is a feature we really need at this point. The situation with types-cryptography is becoming something of a farce. We've also got an increasing number of packages that would benefit from having numpy as a dependency.

  11. ilevkivskyi commented on Sep 19, 2022

    @ilevkivskyi
    Member

    Arbitrary code can be executed during a Python package installation, so if someone adds a malicious package as a dependency, it can cause really big damage, as many of types-... packages have very high install rates (like 100K+ per day).

  12. AlexWaygood commented on Sep 19, 2022

    @AlexWaygood
    Member

    Arbitrary code can be executed during a Python package installation

    That's surely a security risk for any open-source package depending on another open-source package -- is the security hole here that users might not be expecting the installation of a stubs package to install, e.g., numpy or whatever as a side effect?

  13. 20 remaining items

  14. AlexWaygood commented on Oct 21, 2022

    @AlexWaygood
    Member

    I can have a stab at getting mypy_test.py and regr_test.py to work with non-types dependencies. I don't know where to start when it comes to getting pyright and pytype to work, though.

  15. Avasam commented on Oct 22, 2022

    @Avasam
    SponsorCollaborator

    I don't know where to start when it comes to getting pyright and pytype to work, though.

    Sorry if I misunderstand, but isn't it just a matter of installing non-tyshed-stub dependencies from METADATA.toml?

    Couldn't a script be made for that, and re-used. Whether it's in a virtual environment for testing. Or run directly as a CI step before the type checker. (Since Pyright is ran from a github action, we can't tack this on an existing script).

  16. AlexWaygood commented on Oct 22, 2022

    @AlexWaygood
    Member

    Couldn't a script be made for that, and re-used.

    Quite possibly. My point was just that I haven't really looked at pytype_test.py, and don't really know how it works, whereas I know pretty well how mypy_test.py and regr_test.py work :)

  17. added a commit that references this issue on Nov 4, 2022
  18. Avasam commented on Dec 16, 2022

    @Avasam
    SponsorCollaborator

    @AlexWaygood I'd love to see this completed. Is there anything I could help with to free up some of your (and others') time?

  19. AlexWaygood commented on Dec 16, 2022

    @AlexWaygood
    Member

    I'd love to see it completed as well, but just haven't had the time to work on it lately, with all the other typeshed-related things going on :(

    If you fancy taking a look at getting pytype_test.py and/or pyright to work with non-types dependencies, that would be a huge help! I don't think all the scripts/workflows need to be updated in one PR -- we can make changes on the basis of "this would make this script compatible with non-types dependencies, if we had any" for now. PRs updating mypy_test.py and/or regr_test.py would also be very welcome -- I haven't had the time to seriously look at them yet :(

  20. added a commit that references this issue on Dec 16, 2022
  21. AlexWaygood commented on Dec 18, 2022

    @AlexWaygood
    Member

    Okay, so in terms of what's left to do here on typeshed's tests:

  22. Avasam commented on Jan 8, 2023

    @Avasam
    SponsorCollaborator

    For visibility / tracking. There is still one issue left with pytype: #9459 (comment)

    There's possible workarounds (silence the error or ignore the file in pytype), but I think we're currently waiting to see if pytype can provide a non-hacky solution with an update.

  23. AlexWaygood commented on Jan 10, 2023

    @AlexWaygood
    Member

    Non-types dependencies are now finally, officially, Allowed 🥳🥳.

    (But let's maybe wait for the stub-uploader to upload the new versions of types-paramiko and types-pyOpenSSL before we add non-types dependencies to any more packages, just to be on the safe side.)

  24. AlexWaygood commented on Jan 10, 2023

    @AlexWaygood
    Member

    Thanks @hauntsaninja for the PRs to the stub-uploader repo, thanks @Avasam for the PR fixing our pyright tests, and thanks @Avasam and @rchen152 for the PRs fixing our pytype tests!

  25. AlexWaygood commented on Jan 10, 2023

    @AlexWaygood
    Member

    Okay, the new versions of types-paramiko and types-cryptography have been uploaded, and everything seems to be going great!

  26. added a commit that references this issue on Jan 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions