add deprecation warning for remove garbage collector dead code #2253
Conversation
WalkthroughThe changes in the Changes
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
Report bugs in Issues The following are automatically added:
Available user actions:
Supported /retest check runs
Supported labels
|
|
@Ahmad-Hafe Why do you remove this? |
Ahmad-Hafe
force-pushed
the
remove_garbage_collector_dead_code
branch
from
a4bcb62 to
cf9a969
Compare
Ahmad-Hafe
force-pushed
the
remove_garbage_collector_dead_code
branch
from
cf9a969 to
b1d21a6
Compare
Ahmad-Hafe
force-pushed
the
remove_garbage_collector_dead_code
branch
from
b1d21a6 to
20b7a53
Compare
|
@myakove Hi Meni, I added description in the pr details section for the purpose of removing this code |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
ocp_resources/datavolume.py (1)
Line range hint
13-19: Consider documenting the removal of garbage collection supportThe implementation looks good after removing the garbage collection code. Consider adding a class-level comment mentioning that garbage collection support was removed in version 4.14 to help future maintainers understand this design decision.
class DataVolume(NamespacedResource): """ DataVolume object. + + Note: Garbage collection support was removed in version 4.14. """
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ocp_resources/datavolume.py(1 hunks)
🔇 Additional comments (2)
ocp_resources/datavolume.py (2)
Line range hint 270-290: Documentation looks good!
The documentation for wait_for_dv_success is well-structured, includes clear parameter descriptions, and provides a helpful example of using the stop_status_func.
Line range hint 1-400: Verify complete removal of garbage collection code
The garbage collection parameters have been removed as intended. Let's verify there are no remaining references to garbage collection in the codebase.
✅ Verification successful
DataVolume garbage collection parameters have been successfully removed
The search results confirm that all garbage collection related parameters have been removed from the DataVolume class. The only remaining garbage collection references in the codebase are:
- Schema definitions and documentation
- Other resource types like DataImportCron and CDIConfig that have their own garbage collection mechanisms
- General Kubernetes garbage collection documentation and metadata
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to garbage collection in the codebase
# Search for garbage collection related terms
echo "Searching for garbage collection references..."
rg -i "garbage.?collect|delete.?after.?completion|dv.?garbage.?collection.?enabled" \
--type python \
--no-heading
# Search for related annotations in YAML files and tests
echo "Searching for related annotations in YAML and test files..."
rg -i "delete.?after.?completion" \
--type yaml --type python \
--glob "**/*test*.{py,yaml}" \
--no-heading
Length of output: 510
Script:
#!/bin/bash
# Let's try a simpler approach without file type filters
# Search for garbage collection related terms
echo "Searching for garbage collection references..."
rg -i "garbage.?collect|delete.?after.?completion|dv.?garbage.?collection.?enabled"
# Search for related annotations in test files
echo "Searching in test files..."
fd -g "*test*.py" -x rg -i "delete.?after.?completion" {}
Length of output: 29150
|
@Ahmad-Hafe DataVolume is garbage collection no longer supported in the latest OCP-released version? |
|
@myakove DataVolume garbage collection is removed, so all functionality in wrapper is no longer relevant |
only removed in v1.62 |
|
/lgtm |
Ahmad-Hafe
force-pushed
the
remove_garbage_collector_dead_code
branch
from
20b7a53 to
8cc58a3
Compare
Ahmad-Hafe
force-pushed
the
remove_garbage_collector_dead_code
branch
from
a6f5b8c to
57e21f7
Compare
Ahmad-Hafe
force-pushed
the
remove_garbage_collector_dead_code
branch
from
57e21f7 to
3286fd1
Compare
Ahmad-Hafe
force-pushed
the
remove_garbage_collector_dead_code
branch
from
3286fd1 to
95f2b48
Compare
Ahmad-Hafe
force-pushed
the
remove_garbage_collector_dead_code
branch
from
95f2b48 to
6a663b0
Compare
ocp_resources/datavolume.py
Outdated
| @@ -311,6 +314,7 @@ def test_dv(): | |||
| if sample and sample.get("status", {}).get("phase") == self.Status.SUCCEEDED: | |||
| break | |||
| elif sample is None and dv_garbage_collection_enabled: | |||
| warn(gc_deprecatio_warning) | |||
There was a problem hiding this comment.
deprecation should be added to dv_garbage_collection_enabled in docstring and at the beginning of the function:
if dv_garbage_collection_enabled is not None:
warn....
|
/wip |
redhat-qe-bot
changed the title
Ahmad-Hafe
force-pushed
the
remove_garbage_collector_dead_code
branch
from
6a663b0 to
63653e3
Compare
|
/wip cancel |
Ahmad-Hafe
force-pushed
the
remove_garbage_collector_dead_code
branch
from
63653e3 to
4dcafeb
Compare
rh-bot-1
changed the title
| @@ -8,6 +8,9 @@ | |||
| from ocp_resources.persistent_volume_claim import PersistentVolumeClaim | |||
| from ocp_resources.resource import NamespacedResource, Resource | |||
| from timeout_sampler import TimeoutExpiredError, TimeoutSampler | |||
| from warnings import warn | |||
|
|
|||
| gc_deprecatio_warning = ("garbage collector is removed in version v1.62 and going to be deprecated", DeprecationWarning) | |||
| @@ -281,7 +284,7 @@ def wait_for_dv_success( | |||
| Args: | |||
| timeout (int): Time to wait for the DataVolume to succeed. | |||
| failure_timeout (int): Time to wait for the DataVolume to have not Pending/None status | |||
| dv_garbage_collection_enabled (bool, default: False): if True, expect that DV will disappear after success | |||
| dv_garbage_collection_enabled (bool, default: False): garbage collector is removed in version v1.62 and going to be deprecated | |||
There was a problem hiding this comment.
v1.62 refers to what version? CDI/Kubevirt etc? Is it going to be removed or already removed?
Short description:
add deprecation warning for future removing DataVolume garbage collection, as part of Remove dead code and unneeded tests
More details:
DataVolume garbage collection was a feature added in 4.12 in an effort to better integrate with data protection and disaster recovery products. Unfortunately it ended up causing more problems than it solved and was disabled via feature gate in 4.14. We still have the feature gate and code and tests for a feature we don't intend anyone
What this PR does / why we need it:
this code is not going to be used in the wrapper in future, so adding deprecation warning
Which issue(s) this PR fixes:
this fixe's and modify the exsists code and prepare warning for deprecation warning
Special notes for reviewer:
https://issues.redhat.com/browse/CNV-53179
Bug:
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Documentation