search

pre-commit-ci / issues

public issues for https://pre-commit.ci
16 stars 3 forks source link

Document the "no network during runtime" restriction #139

Closed consideRatio closed 1 year ago

consideRatio commented 1 year ago

I love pre-commit and I've greatly appreciated pre-commit.ci and started adopting it across the https://github.com/jupyterhub and https://github.com/dask organizations I'm part of. I thought I had learned most things there was to learn about it and planned around how it could be used.

After a lot of effort into a plan I ran into something that was very unexpected for me, the networking limitation described in https://github.com/pre-commit-ci/issues/issues/55#issuecomment-822481997:

this is intentional, network is not allowed at runtime (for now, it may be allowed in the future in paid tiers) as it is prone to abuse

Looking at https://pre-commit.ci/ I did not find any indication of this. I suggest with this issue that this restriction is made more visible. It would in my mind be sufficient if it was findable from the landing page at https://pre-commit.ci/, for example via a link to "restrictions", "limitations", or maybe even directly written out there.

Thank you @asottile and everyone else involved to bring up this great service!

PS: As a maintainer of the service https://mybinder.org that provides compute for free, I think the limitation is very understandable - we invest significant volunteer time to push back on cryptomining abuse etc. I wouldn't want to see pre-commit maintainers need to invest time in that way.

asottile commented 1 year ago

duplicates #55 -- I don't plan on documenting it in that way but instead in improved error messaging directly in the job. it'll also be a pro tier feature at some point

consideRatio commented 1 year ago

This makes me sad to hear. I invested a lot of time and effort into upstreaming fixes into a pre-commit hook for it to be able to be used with the excellent pre-commit.ci feature of pushing fixes to a PR. It all became irrelevant work by me and the people helping me in this pursuit due to me not knowing this limitation.

asottile commented 1 year ago

it's pretty unusual to need network to run linters or code formatters -- without knowing more I suspect you're trying to do too much in a pre-commit hook or you can move things to install time

HippocampusGirl commented 1 year ago

I would love to use pre-commit.ci, it is truly a wonderful tool. One of my pre-commit hooks unfortunately is pip-compile, since we have a lot of dependencies that need to be managed.

Sadly, without network access, pip-compile cannot check my dependencies, and I get pre-commit.ci errors. For example, here.

Have you considered having an allowlist of pypi and GitHub IP-Addresses that can be accessed through the network? This way, the potential for abuse would be minimal.

asottile commented 1 year ago

not happening for free, sorry. your suggestion still opens the door for miner abuse

also pip-compile does not belong in pre-commit -- it's far too slow

consideRatio commented 1 year ago

I suspect you're trying to do too much in a pre-commit hook or you can move things to install time

also pip-compile does not belong in pre-commit -- it's far too slow

I think its quite problematic that the "trying to do too much" and "does not belong" parts is something people find out by practical experience with pre-commit.ci rather than being able to plan ahead for it. I understand that projects have failed to document something of relevance, but to intentionally choose to not document something like this me feel very uneasy.

asottile commented 1 year ago

people don't read the docs anyway

HippocampusGirl commented 1 year ago

Hi @asottile, Thank you for the quick response! I understand now that pre-commit.ci is not the tool that we want to use, and will continue using GitHub Actions for my linting and pip-compile purposes.

I agree with @consideRatio that having a note in the docs would have saved me some time. I also agree with your point that few people read the full docs, but it might still be nice to have a bullet point somewhere for those that do ;-)

I understand that pip-compile is a bit of an unusual choice for a pre-commit hook, but it has saved us quite a bit of hassle over time to have all of the code checks in one place. I have also configured the hook so that it will only run after modification to the dependencies, which means that we don't incur that overhead in every commit, but only when necessary.

jpmckinney commented 1 year ago

people don't read the docs anyway

I read docs :)

I use pip-compile in pre-commit – I'm very rarely changing my requirements files, so it's skipped for 99% of commits.

I'd been seeing this error but didn't bother trying to find the cause until now. The error output didn't contain anything suggesting it was a limitation by design: https://results.pre-commit.ci/run/github/191574112/1685135502.s8wdt_KnQSCB8WpoOMjysg

I agree with the limitation, but it could be documented.