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

Update pre-commit hook to avoid committing zip file #46459

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

SaumilPatel03
Copy link
Contributor


What was wrong: Committed ZIP files are hard to review and can pose security risks.

What was changed: added a Python pre-commit hook to prevent their accidental commit.

closes: #46449

Added a link to PEP 668 in the virtual environment section
Add pre-commit hook to prevent committing .zip files
Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. There are unrelated changes in there - describing documentation about development environment. If you want to submit it, you should have separate PR about it.
  2. I think such precommit makes no sense. It can be done by just setting .gitignore at top level to include .zip files - there is absolutely no need to have pre-commit for that and it is harmful actually. We might want to commit some .zip files for tests for example so I don't even think it is a good idea in the first place

@eladkal
Copy link
Contributor

eladkal commented Feb 5, 2025

But right now we have zip files
tests/dags/test_zip.zip I think par to of the task is to drop what we have now and replace it with creation of the zip on the fly.

@potiuk
Copy link
Member

potiuk commented Feb 5, 2025

But right now we have zip files tests/dags/test_zip.zip I think par to of the task is to drop what we have now and replace it with creation of the zip on the fly.

Yes. But I believe we do not need pre-commit for that - .gitignore should be enough. And even if we want to have .zip check the precommit should be just false command if .*.zip files match - there is no need to have python for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Get rid of *.zip in tests cases
4 participants