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

Fix failing pre-commit hook fails on directories #17

Merged
merged 2 commits into from
Feb 7, 2023

Conversation

letmerecall
Copy link
Member

@letmerecall letmerecall commented Feb 7, 2023

Purpose

Why this PR?
When the change is in directory, pre-commit hook fails with error.

Adding a symlink in text/symlink raise this error:

Check Yaml...........................................(no files to check)Skipped
Format code (black)......................................................Passed
Sort imports (isort).....................................................Passed
Check for PII............................................................Failed
- hook id: pii-check
- duration: 0.11s
- exit code: 1

Traceback (most recent call last):
  File "/home/guydumais/.cache/pre-commit/repomg4xos05/py_env-python3.8/bin/pii_check", line 8, in <module>
    sys.exit(main())
  File "/home/guydumais/.cache/pre-commit/repomg4xos05/py_env-python3.8/lib/python3.8/site-packages/pii_check/pii_check_hook.py", line 155, in main
    check_for_pii(args.url, API_KEY, enabled_entity_list)
  File "/home/guydumais/.cache/pre-commit/repomg4xos05/py_env-python3.8/lib/python3.8/site-packages/pii_check/pii_check_hook.py", line 104, in check_for_pii
    flagged = get_flagged_lines(files)
  File "/home/guydumais/.cache/pre-commit/repomg4xos05/py_env-python3.8/lib/python3.8/site-packages/pii_check/pii_check_hook.py", line 36, in get_flagged_lines
    with open(file, "r") as fp:
IsADirectoryError: [Errno 21] Is a directory: 'text/symlink'

The error is a result of checking the PII_CHECK flag on changed files and dirs.

Description

What is changed?

  • Skip PII_CHECK flag check for directories.
  • Add unittest for get_flagged_lines()

How has this been tested?

  • Running unittest.
  • Installing updated version of pre-commit hook and testing on the failing case.

Checklist

  • No linting error?
  • No dead code?

@letmerecall letmerecall requested review from guyd and koryf February 7, 2023 16:36
@letmerecall letmerecall changed the title [DEID-1376] Fix failing pre-commit hook fails on directories Fix failing pre-commit hook fails on directories Feb 7, 2023
Copy link
Contributor

@guyd guyd left a comment

Choose a reason for hiding this comment

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

LGTM

@letmerecall letmerecall merged commit 3db5d4a into dev Feb 7, 2023
@letmerecall letmerecall deleted the DEID-1376-pre-commit-hook-fails-on-directories branch February 7, 2023 17:56
letmerecall added a commit that referenced this pull request Feb 8, 2023
* Updated README to include blocked-list and restructured

* Adding minor suggestion by Guy

Co-authored-by: Guy Dumais <dumais.guy@gmail.com>

* update the readme with the blocked-list details

* update with new REST route

* Update README.md

Co-authored-by: koryf <101284003+koryf@users.noreply.github.com>

* Update README.md

Co-authored-by: koryf <101284003+koryf@users.noreply.github.com>

* keeping the old REST path until 3.0 is released.

* Update pii_dict format according to deid 3.0.0beta3

* Fix failing pre-commit hook fails on directories (#17)

* Add test for get flagged lines

* Skip PII flag check for directories

---------

Co-authored-by: ketakipai <ketaki@private-ai.com>
Co-authored-by: ketakipai <110412492+ketakipai@users.noreply.github.com>
Co-authored-by: Guy Dumais <dumais.guy@gmail.com>
Co-authored-by: Guy Dumais <guy@private-ai.com>
Co-authored-by: koryf <101284003+koryf@users.noreply.github.com>
guyd added a commit that referenced this pull request Feb 24, 2023
* Updated README to include blocked-list and restructured

* Adding minor suggestion by Guy

Co-authored-by: Guy Dumais <dumais.guy@gmail.com>

* update the readme with the blocked-list details

* update with new REST route

* Update README.md

Co-authored-by: koryf <101284003+koryf@users.noreply.github.com>

* Update README.md

Co-authored-by: koryf <101284003+koryf@users.noreply.github.com>

* keeping the old REST path until 3.0 is released.

* Update pii_dict format according to deid 3.0.0beta3

* Fix failing pre-commit hook fails on directories (#17)

* Add test for get flagged lines

* Skip PII flag check for directories

* DEID-1458 - fix parametrization of the hook and many other aspects (#21)

* DEID-1458 - fix parametrization of the hook and many other aspects

* Update tests/test_get_flagged_lines.py

Co-authored-by: Girish Sharma <girishsharma001@gmail.com>

* DEID-1458 - rename to a more general filename

* DEID-1458 - add unidiff to parse the diff and extract the text to check

* DEID-1458 - add missing dependency

* DEID-1458 - output PII location as a clickable link in Visual Studio code

* DEID-1458 - remove debugging code

---------

Co-authored-by: Girish Sharma <girishsharma001@gmail.com>

---------

Co-authored-by: ketakipai <ketaki@private-ai.com>
Co-authored-by: ketakipai <110412492+ketakipai@users.noreply.github.com>
Co-authored-by: koryf <101284003+koryf@users.noreply.github.com>
Co-authored-by: letmerecall <girishsharma001@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants