Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants