-
Notifications
You must be signed in to change notification settings - Fork 5
Log no-lint-disabled CI #353
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ 36/36 passed, 4 skipped, 5m39s total Running from acceptance #480 |
464b4a6
to
209fd98
Compare
209fd98
to
844614d
Compare
asnare
previously requested changes
Nov 19, 2024
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.
Looks fine, although some style nits.
08644d4
to
cc444e4
Compare
pritishpai
approved these changes
Feb 27, 2025
JCZuurmond
added a commit
that referenced
this pull request
Feb 27, 2025
* Explicitly install Python 3.12 before running fmt in CI ([#358](#358)). In this release, we have made several enhancements to our open-source library. Firstly, we have explicitly installed Python 3.12 in the CI workflow before running the formatting task, ensuring consistency and predictability in the CI environment. This allows developers to verify the formatting locally with the same Python version. Additionally, we have added caching for pip and the pyproject.toml file to improve build performance. We have also updated the `backends.py` file to modify the `save_table` function, changing the way a specific SQL statement is constructed. Furthermore, we have made changes to the `structs.py` file, updating error messages to use double quotes and improving their readability. These changes address issues reported in [#357](#357) and improve the overall compatibility, local verification, and code readability of the library. * Log no-lint-disabled CI ([#353](#353)). In this open-source library release, the CI workflow has been updated to include new code that checks for disables of linting in new code pushed to the main branch. The code generates a diff of the new code and searches for instances where pylint has been explicitly disabled using '# pylint: disable'. If any disables are found, the number is stored in the variable CHEAT. If CHEAT is not equal to 0, the CI workflow will fail and the push will be rejected. Additionally, echo statements have been included to print the code diff and the number of cheats found for debugging and visibility purposes. This update ensures that all new code adheres to the required linting standards and promotes high-quality code contributions to the library. * Release v0.14.2 ([#361](#361)). 0.14.2 release introduces resource name validation for `ucx` project's dashboard tile IDs, restricting names to alphanumeric characters, hyphens, and underscores. A new method `_is_valid_resource_name` checks if a name is valid, and the `TileMetadata` class is updated with a `validate` method to ensure its `id` attribute adheres to the new naming convention and is not empty. `Tile`, `Section`, and `Dashboard` classes are also updated to call the `TileMetadata` instance's `validate` method. This release adds tests for validation functionality, ensures compatibility with Databricks SDK v0.38.0, and updates the GitHub Actions workflow to utilize a protected runner group with the label `linux-ubuntu-latest` for enhanced control and security. Additionally, 'backends.py', 'model.py', 'polymorphism.py', and 'structs.py' files are improved for better code readability, maintainability, and reliability, addressing potential SQL injection issues. * Removed the no cheat CI check ([#362](#362)). In the latest release, we have removed the `no-lint-disabled` job from our CI workflow. This job previously checked for the presence of "# pylint: disable" comments in newly committed code and failed the CI pipeline if any such comments were found. However, due to frequent false positives, the job was causing unnecessary failures in the pipeline. As an alternative, we recommend implementing a more robust cheat detection mechanism in the future, similar to the one used in the ucx project, which utilizes a Python script to verify that no cheats are being used. This change simplifies the CI workflow, reduces false positives, and allows for the addition of a more sophisticated cheat detection mechanism in the future. For more information, please refer to the provided GitHub Actions link. * Updated runner ([#360](#360)). In this update, we have improved the security and control of our GitHub Actions workflow responsible for publishing releases. Previously using the default `ubuntu-latest` runner, we have now switched to a more secure option belonging to the `databrickslabs-protected-runner-group` group with the `linux-ubuntu-latest` label. This change enhances environment control while maintaining compatibility with the previous setup. Additionally, the `environment` field has been updated to `release`, and we have preserved the existing permissions settings for authenticating to PyPI via OIDC and signing the release's artifacts with `sigstore-python`. * Validate resource names ([#357](#357)). This commit introduces a validation for resource names in the `TileMetadata` class to adhere to a new naming convention. Names must only contain alphanumeric characters, hyphens, or underscores. A new private method `_is_valid_resource_name` is added to the `TileMetadata` class to check for valid names. The `TileMetadata` class now includes a `validate` method, raising a `ValueError` if the name is invalid. The `Tile` class has also been updated to raise a `ValueError` if the `TileMetadata` is invalid. The `__post_init__` method of `TileMetadata` now replaces any occurrence of ".filter" with `_filter` in the id to ensure validity. Resource names, specifically for dashboard tile IDs, are now validated to comply with the new naming convention, addressing issues [#355](#355), [#356](#356), and [#354](#354). The validation checks for Tile IDs with non-alphanumeric characters, duplicate query or widget IDs, and empty tile IDs. Tile classes now validate the tile name, raising a ValueError if the name contains spaces. Additionally, the tests for MarkdownTile have been updated to ensure that the tile content is loaded from a markdown file and that invalid markdown tile content will now raise a ValueError. This change adds validation for resource names, enforces stricter requirements for tile IDs and tile names, and improves the overall robustness of the tile metadata handling.
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
CI failed and we do not know why