Skip to content
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

fix:cache_gui_files #236

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

fix:cache_gui_files #236

wants to merge 2 commits into from

Conversation

JarbasAl
Copy link
Member

@JarbasAl JarbasAl commented Sep 18, 2024

companion to OpenVoiceOS/ovos-gui#53 and OpenVoiceOS/ovos-bus-client#120

this should be merged first as it does not require the others to work and does not break anything in the wild

Summary by CodeRabbit

  • New Features

    • Improved handling of GUI resources for enhanced performance and compatibility.
    • Introduced a caching mechanism for GUI files to ensure the latest resources are utilized.
  • Bug Fixes

    • Added compatibility layer for retrieving UI directories to maintain functionality.
    • Updated version constraints for the ovos_bus_client dependency to allow for newer features and bug fixes.
  • Chores

    • Removed outdated test class related to GUI interface validation.

companion to OpenVoiceOS/ovos-gui#53 and OpenVoiceOS/ovos-bus-client#120

this should be merged first as it does not require the others to work and does not break anything in the wild
Copy link

coderabbitai bot commented Sep 18, 2024

Walkthrough

The changes focus on enhancing the OVOSSkill class's handling of GUI resources. A new method, _cache_gui_files, is introduced to manage GUI file caching more effectively, ensuring the latest resources are used. The initialization of the GUIInterface is updated to include a compatibility layer for retrieving UI directories, maintaining backward compatibility. Additionally, the code now accommodates different styles of resource organization, specifically checking for a "qt5" subfolder, and provides warnings for incorrect setups. The version constraint for the ovos_bus_client dependency is also updated to allow for newer versions.

Changes

Files Change Summary
ovos_workshop/skills/ovos.py Added _cache_gui_files method to manage GUI file caching; modified GUIInterface initialization for compatibility; updated resource handling for "qt5" subfolder organization.
requirements/requirements.txt Modified ovos_bus_client version constraint from >=0.0.8,<1.0.0 to >=0.0.8,<2.0.0.
test/unittests/skills/test_base.py Removed TestSkillGui class and its methods, eliminating GUI interface testing for skills.

Poem

In the burrow where skills do thrive,
New GUI paths help us to strive.
With caching done, and warnings bright,
Our rabbit hearts leap with delight! 🐇✨
Let’s hop along, with joy we sing,
For every change, new hope they bring!


Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between bf1acd6 and d988c36.

Files selected for processing (2)
  • requirements/requirements.txt (1 hunks)
  • test/unittests/skills/test_base.py (0 hunks)
Files not reviewed due to no reviewable changes (1)
  • test/unittests/skills/test_base.py
Additional comments not posted (1)
requirements/requirements.txt (1)

2-2: Verify compatibility with ovos_bus_client v1.x.x.

The expanded version constraint for ovos_bus_client now allows for versions 1.x.x. Ensure that these versions have been tested for compatibility with the current codebase.

Run the following script to check if the 1.x.x versions introduce any breaking changes:

Verification successful

Approve version constraint change for ovos_bus_client.

The expanded version constraint for ovos_bus_client (>=0.0.8,<2.0.0) appears to be safe based on the current usage in the codebase. There are no visible version-specific dependencies or checks that would indicate incompatibility with v1.x.x versions. However, to ensure smooth operation:

  • Conduct thorough testing with v1.x.x versions of ovos_bus_client to verify full compatibility.
  • Monitor for any unexpected behavior or errors that may arise from using newer versions of ovos_bus_client.
  • Keep an eye on the changelog or release notes of ovos_bus_client for any breaking changes in future releases.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify compatibility with `ovos_bus_client` v1.x.x.

# Test: Search for `ovos_bus_client` usage. Expect: No breaking changes in v1.x.x.
rg --type python -A 5 $'ovos_bus_client'

Length of output: 70


Script:

#!/bin/bash
# Description: Find Python files and search for ovos_bus_client usage

# Find Python files
python_files=$(fd -e py)

# Search for ovos_bus_client in Python files
if [ -n "$python_files" ]; then
    echo "Searching for ovos_bus_client usage in Python files:"
    echo "$python_files" | xargs rg -n -A 5 'ovos_bus_client'
else
    echo "No Python files found in the repository."
fi

# Additionally, search for ovos_bus_client in all files (as a fallback)
echo "Searching for ovos_bus_client in all files:"
rg -n -A 5 'ovos_bus_client'

Length of output: 36339


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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    -- I pushed a fix in commit <commit_id>, please review it.
    -- Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    -- @coderabbitai generate unit testing code for this file.
    -- @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    -- @coderabbitai generate interesting stats about this repository and render them as a table.
    -- @coderabbitai read src/utils.ts and generate unit testing code.
    -- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    -- @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot added fix and removed fix labels Sep 18, 2024
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Outside diff range and nitpick comments (1)
ovos_workshop/skills/ovos.py (1)

2739-2762: The _cache_gui_files logic looks good! Consider a minor refactor to reduce nesting.

The function correctly handles caching the skill's GUI resources, supporting both the new and old style directory structures. Removing the existing cache before copying new resources ensures the cache stays up to date.

To improve readability, consider extracting the cache copying logic into a separate function to reduce nesting:

def _copy_gui_files(self, base_dir, output_dir):
    # backwards compat - TODO update once we support more than QT5
    if os.path.isdir(f"{base_dir}/qt5"):
        # new style, multi GUI frameworks supported  
        shutil.copytree(base_dir, output_dir)
    else:
        # old style, QT5 only
        LOG.warning(f"GUI resources moved to platform specific subfolder, "
                    f"please move contents from {base_dir} to 'qt5' subfolder")
        shutil.copytree(base_dir, f"{output_dir}/qt5")

def _cache_gui_files(self):
    base = f"{self._skill.root_dir}/ui"
    if not os.path.isdir(base):
        LOG.debug(f"{self.skill_id} has no GUI resources: {base}")
        return

    GUI_CACHE_PATH = get_xdg_cache_save_path('ovos_gui')
    output_path = f"{GUI_CACHE_PATH}/{self.skill_id}"
    
    if os.path.exists(output_path):
        LOG.info(f"Removing existing {self.skill_id} cached GUI resources before updating")
        shutil.rmtree(output_path)
        
    self._copy_gui_files(base, output_path)
    LOG.debug(f"Copied {self.skill_id} resources from {base} to {output_path}")
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between e4b1677 and bf1acd6.

Files selected for processing (1)
  • ovos_workshop/skills/ovos.py (5 hunks)
Additional context used
Ruff
ovos_workshop/skills/ovos.py

2735-2735: Do not use bare except

(E722)

ovos_workshop/skills/ovos.py Show resolved Hide resolved
companion to OpenVoiceOS/ovos-gui#53 and OpenVoiceOS/ovos-bus-client#120

this should be merged first as it does not require the others to work and does not break anything in the wild
Copy link

codecov bot commented Sep 18, 2024

Codecov Report

Attention: Patch coverage is 50.00000% with 12 lines in your changes missing coverage. Please review.

Project coverage is 49.09%. Comparing base (7c02206) to head (d988c36).
Report is 14 commits behind head on dev.

Files with missing lines Patch % Lines
ovos_workshop/skills/ovos.py 50.00% 12 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #236      +/-   ##
==========================================
- Coverage   53.41%   49.09%   -4.32%     
==========================================
  Files          37       37              
  Lines        4362     4379      +17     
==========================================
- Hits         2330     2150     -180     
- Misses       2032     2229     +197     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant