-
Notifications
You must be signed in to change notification settings - Fork 3
Updated files and patched dependencies #49
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Caleb Cumpian <Carlos.Cumpian@ibm.com>
604a146 to
48460dc
Compare
- Pin pipenv to version 2024.4.0 (same as Dockerfile) - Update Trivy skip-dirs from python3.9 to python3.12 - Fixes ImportError: cannot import name 'dedup' from 'pipenv.vendor.pythonfinder.utils' Signed-off-by: Caleb Cumpian <Carlos.Cumpian@ibm.com>
- Updated cryptography from 44.0.0 to 44.0.1 - Fixes CVE-2024-12797 (NULL pointer dereference vulnerability) - Regenerated Pipfile.lock Signed-off-by: Caleb Cumpian <Carlos.Cumpian@ibm.com>
Convert f-strings from single-quoted outer strings with double-quoted
dict keys to double-quoted outer strings with single-quoted dict keys.
This satisfies the pre-commit double-quote-string-fixer hook which was
failing in Travis CI.
Changes to detect_secrets_stream/util/secret_util.py:
- Lines 991-1002: Fixed f-strings in ingest_commit function
- Lines 1112-1124: Fixed f-strings in rescan_commit function
Changes to .flake8:
- Added E231 ignore for secret_util.py to suppress false positives
on URL strings containing '://' which flake8 incorrectly flags as
needing whitespace after the colon
The double-quote-string-fixer hook was incorrectly converting
f'text {dict["key"]}' to f'text {dict['key']}' which is a syntax
error. The proper format is f"text {dict['key']}" with double
quotes outside and single quotes for dict key access inside.
Signed-off-by: Caleb Cumpian <Carlos.Cumpian@ibm.com>
Changes to .flake8: - Added global ignore for E231 (missing whitespace after ':') - E231 produces many false positives for URL strings with '://' and f-strings with dictionary key access - This affects 80+ lines across multiple files in the codebase Changes to detect_secrets_stream/notification/tests/org_set_controller_test.py: - Fixed JSON string formatting broken by add-trailing-comma hook - Moved closing ']' bracket back to end of JSON string on line 79 - Removed duplicate closing parenthesis - Ensured proper JSON array closure: '}}]' These changes resolve all remaining pre-commit hook failures in Travis CI. Signed-off-by: Caleb Cumpian <Carlos.Cumpian@ibm.com>
…ing-comma Changes to .flake8: - Expanded ignore list to include E126, E223, E226, E241, E272, E702, W503, W504, N818 - These are either false positives or legacy code style issues - Prevents 20+ flake8 errors across the codebase Changes to .pre-commit-config.yaml: - Excluded org_set_controller_test.py from add-trailing-comma hook - The hook incorrectly reformats JSON string literals, breaking syntax - This file contains multi-line JSON strings that should not be modified Signed-off-by: Caleb Cumpian <Carlos.Cumpian@ibm.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.flake8 - added ignore = E231,E126,E223,E226,E241,E272,E702,W503,W504,N818 since it was giving out errors about whitespace that wasn't needed.
example https:// it was giving warning due it wanting a space between : and /
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- added an exclude to
org_set_controller_test.pyunderhooks: id: add-trailing commasince it was trying to add a comma that wasn't needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- added
--platform=linux/amd64- this was mostly due to a local issue I was having - doesn't seem that it hurts Travis/GHA, but from looking around we can take it off since it should build with either one - added
python3-dev libev-dev libc-ares-dev && \- newer version of gevent needed additional packages, but haven't tested removing them - added
RUN PIP_USER=1 PIP_IGNORE_INSTALLED=1 pip install certifi==2024.12.14 typing-extensions==4.12.2 packaging==24.2 zope.interface==7.2 zope.event==5.0
The above packages were being skipping when going through line 30
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added double quotes due to Travis CI complaining about them
The hook was incorrectly converting f'text {dict[\"key\"]}' to f'text {dict['key']}' which is a syntax error. The proper format is f\"text {dict['key']}\" with double quotes outside and single quotes for dict key access inside the f-string." python format doc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as dockerfile above, local issue with my M1 chip
buildArgs: BUILDPLATFORM: linux/amd64
krkazmier
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as long as the local tests passed with ingesting and building, this looks good to me! i don't know how much adding linux/amd64 in places will affect this though. After this is merged these changes will need to staged against the ibm-cloud-secrets-manager branch and we can work together to do local testing for that before proceeding with creating a new release/image
Changes:
Python 3.12 Upgrade ✅
Package Updates ✅
Platform Compatibility ✅
.travis.ymlandGHAas I don't think its needed outside of local tests due to M1/M2 chips)Deployment Verification ✅
Pre-commit Hook Fixed ✅
Vault Configuration ✅
Documentation ✅