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)
⏰ Context from checks skipped due to timeout of 90000ms (2)
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
4dcafeb to
b4a60f8
Compare
Ahmad-Hafe
force-pushed
the
remove_garbage_collector_dead_code
branch
from
b4a60f8 to
5ddf5a1
Compare
ocp_resources/datavolume.py
Outdated
| gc_deprecation_warning = ( | ||
| "garbage collector is removed in version v1.62 and going to be deprecated", | ||
| DeprecationWarning, | ||
| ) |
There was a problem hiding this comment.
only used once, no need to save
Ahmad-Hafe
force-pushed
the
remove_garbage_collector_dead_code
branch
from
4068b81 to
777ea8e
Compare
Ahmad-Hafe
force-pushed
the
remove_garbage_collector_dead_code
branch
from
777ea8e to
5397cde
Compare
Ahmad-Hafe
force-pushed
the
remove_garbage_collector_dead_code
branch
from
5397cde to
d170729
Compare
Ahmad-Hafe
force-pushed
the
remove_garbage_collector_dead_code
branch
from
d170729 to
bda5305
Compare
Ahmad-Hafe
force-pushed
the
remove_garbage_collector_dead_code
branch
from
bda5305 to
3483838
Compare
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
Documentation