Update config.py #173
Update config.py #173
Conversation
fix deprecation warning
WalkthroughThe changes in the pull request primarily involve modifications to the Changes
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 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
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #173 +/- ##
=====================================
Coverage ? 0.00%
=====================================
Files ? 10
Lines ? 877
Branches ? 0
=====================================
Hits ? 0
Misses ? 877
Partials ? 0 ☔ View full report in Codecov by Sentry. |
![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
🧹 Outside diff range and nitpick comments (2)
ovos_config/config.py (2)
Line range hint
76-89: LGTM: Appropriate deprecation ofload_config_stack.The deprecation of
load_config_stackis well-implemented:
- A clear warning message is logged.
- The method still functions as before, allowing for a gradual transition.
- The alternative method
load_all_configsis suggested.Consider adding a deprecation decorator (e.g.,
@deprecated) if available in your codebase, to provide additional visibility and potentially automate deprecation handling.🧰 Tools
🪛 Ruff
267-267:
ovos_utils.network_utils.is_connectedimported but unusedRemove unused import:
ovos_utils.network_utils.is_connected(F401)
268-268: Undefined name
is_connected_http(F821)
271-271: Do not use bare
except(E722)
Line range hint
401-404: Improve deprecation ofclear_cachemethod.The deprecation of
clear_cacheis appropriate given that there's no cache anymore. However, there are a couple of points to consider:
- The method still calls
Configuration.updated(message), which might be unnecessary.- There's no clear indication to users about what they should do instead of calling this method.
Consider the following improvements:
- If
Configuration.updated(message)is truly unnecessary, remove it:@staticmethod def clear_cache(message=None): """DEPRECATED - there is no cache anymore """ - Configuration.updated(message) pass
- Add a deprecation warning with guidance for users:
@staticmethod def clear_cache(message=None): """DEPRECATED - there is no cache anymore """ + import warnings + warnings.warn("clear_cache is deprecated and does nothing. Configuration updates are now handled automatically.", DeprecationWarning, stacklevel=2) passThis will provide clearer guidance to users about the deprecation and what, if anything, they need to do instead.
🧰 Tools
🪛 Ruff
267-267:
ovos_utils.network_utils.is_connectedimported but unusedRemove unused import:
ovos_utils.network_utils.is_connected(F401)
268-268: Undefined name
is_connected_http(F821)
271-271: Do not use bare
except(E722)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- ovos_config/config.py (1 hunks)
🧰 Additional context used
🪛 Ruff
ovos_config/config.py
268-268: Undefined name
is_connected_http(F821)
🔇 Additional comments (1)
ovos_config/config.py (1)
265-270:⚠️ Potential issueUpdate internet connectivity check and resolve import issues.
The change from
is_connected()tois_connected_http()appears to address a deprecation, which is good. However, there are a few issues to address:
- The import statement for
is_connectedis now unused and should be removed.is_connected_http()is not imported or defined, which will cause a NameError.- The TODO comment suggests intermittent import issues, which should be investigated.
To resolve these issues:
- Remove the unused import:
- from ovos_utils.network_utils import is_connected
- Import the new function:
+ from ovos_utils.network_utils import is_connected_http
- Investigate and resolve the intermittent import failures mentioned in the TODO comment.
To ensure the
is_connected_httpfunction is available, run:This will help verify the existence and location of the new function.
🧰 Tools
🪛 Ruff
267-267:
ovos_utils.network_utils.is_connectedimported but unusedRemove unused import:
ovos_utils.network_utils.is_connected(F401)
268-268: Undefined name
is_connected_http(F821)
forgot the import
fix deprecation warning
Summary by CodeRabbit
load_config_stackandclear_cachemethods as deprecated; users are encouraged to useload_all_configsinstead.