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

feat(performance): configurable filter #29

Merged
merged 5 commits into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/build_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@ jobs:
uses: neongeckocom/.github/.github/workflows/python_build_tests.yml@master
with:
test_pipaudit: true
pipaudit_ignored: "GHSA-r9hx-vwmv-q579 PYSEC-2022-43012 GHSA-j8r2-6x86-q33q"
pipaudit_ignored: "GHSA-r9hx-vwmv-q579 PYSEC-2022-43012 GHSA-j8r2-6x86-q33q GHSA-9wx4-h78v-vm56"
8 changes: 4 additions & 4 deletions .github/workflows/install_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@ jobs:
strategy:
max-parallel: 2
matrix:
python-version: [ 3.7, 3.8, 3.9, "3.10" ]
python-version: [3.8, 3.9, "3.10", "3.11", "3.12"]
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: actions/checkout@v4
- name: Setup Python
uses: actions/setup-python@v1
uses: actions/setup-python@v5
with:
python-version: ${{ matrix.python-version }}
- name: Install Build Tools
Expand All @@ -31,4 +31,4 @@ jobs:
python setup.py bdist_wheel
- name: Install package
run: |
pip install .[all]
pip install .[all]
45 changes: 23 additions & 22 deletions .github/workflows/unit_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,42 +4,42 @@ on:
branches:
- dev
paths-ignore:
- 'ovos_messagebus/version.py'
- 'examples/**'
- '.github/**'
- '.gitignore'
- 'LICENSE'
- 'CHANGELOG.md'
- 'MANIFEST.in'
- 'readme.md'
- 'scripts/**'
- "ovos_messagebus/version.py"
- "examples/**"
- ".github/**"
- ".gitignore"
- "LICENSE"
- "CHANGELOG.md"
- "MANIFEST.in"
- "readme.md"
- "scripts/**"
push:
branches:
- master
paths-ignore:
- 'ovos_messagebus/version.py'
- 'requirements/**'
- 'examples/**'
- '.github/**'
- '.gitignore'
- 'LICENSE'
- 'CHANGELOG.md'
- 'MANIFEST.in'
- 'readme.md'
- 'scripts/**'
- "ovos_messagebus/version.py"
- "requirements/**"
- "examples/**"
- ".github/**"
- ".gitignore"
- "LICENSE"
- "CHANGELOG.md"
- "MANIFEST.in"
- "readme.md"
- "scripts/**"
workflow_dispatch:

jobs:
unit_tests:
strategy:
max-parallel: 2
matrix:
python-version: [ 3.7, 3.8, 3.9]
python-version: [3.8, 3.9, "3.10", "3.11", "3.12"]
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: actions/checkout@v4
- name: Set up python ${{ matrix.python-version }}
uses: actions/setup-python@v2
uses: actions/setup-python@v5
with:
python-version: ${{ matrix.python-version }}
- name: Install System Dependencies
Expand All @@ -50,6 +50,7 @@ jobs:
- name: Install core repo
run: |
pip install .

# TODO: Implement unit tests
# - name: Install test dependencies
# run: |
Expand Down
8 changes: 6 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,12 @@ under mycroft.conf
"ssl": false,
// in mycroft-core all skills share a bus, this allows malicious skills
// to manipulate it and affect other skills, this option ensures each skill
// gets it's own websocket connection
"shared_connection": true
// gets its own websocket connection
"shared_connection": true,
// filter out messages of certain types
mikejgray marked this conversation as resolved.
Show resolved Hide resolved
"filter": false,
// which messages to filter if filter is enabled
"filter_ogs": ["gui.status.request", "gui.page.upload"]
mikejgray marked this conversation as resolved.
Show resolved Hide resolved
}
}
```
52 changes: 33 additions & 19 deletions ovos_messagebus/event_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,31 +35,45 @@ def __init__(self, application, request, **kwargs):
def on(self, event_name, handler):
self.emitter.on(event_name, handler)

@property
def filter(self) -> bool:
return Configuration().get("websocket", {}).get("filter", False)

@property
def filter_ogs(self) -> list:
return Configuration().get("websocket", {}).get("filter_ogs", ["gui.status.request", "gui.page.upload"])

@property
def max_message_size(self) -> int:
return Configuration().get("websocket", {}).get("max_msg_size", 10) * 1024 * 1024

def on_message(self, message):
if not self.filter:
Copy link
Member

Choose a reason for hiding this comment

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

This could be simplified making if self.filter the conditional and leaving the common code here outside of the conditional

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair point. I put it on top because I wanted the default use case to be the first bit of code you read, rather than having to search down the method to find out what it does in a normal scenario.

try:
self.emitter.emit(message)
except Exception as e:
LOG.exception(e)
traceback.print_exc(file=sys.stdout)
pass
else:
try:
deserialized_message = Message.deserialize(message)
except Exception:
return

if deserialized_message.msg_type not in self.filter_ogs:
LOG.debug(deserialized_message.msg_type +
f' source: {deserialized_message.context.get("source", [])}' +
f' destination: {deserialized_message.context.get("destination", [])}\n'
f'SESSION: {SessionManager.get(deserialized_message).serialize()}')

try:
deserialized_message = Message.deserialize(message)
except Exception:
return

filter_ogs = ["gui.status.request", "gui.page.upload"]
if deserialized_message.msg_type not in filter_ogs:
LOG.debug(deserialized_message.msg_type +
f' source: {deserialized_message.context.get("source", [])}' +
f' destination: {deserialized_message.context.get("destination", [])}\n'
f'SESSION: {SessionManager.get(deserialized_message).serialize()}')

try:
self.emitter.emit(deserialized_message.msg_type,
deserialized_message)
except Exception as e:
LOG.exception(e)
traceback.print_exc(file=sys.stdout)
pass
try:
self.emitter.emit(deserialized_message.msg_type,
deserialized_message)
except Exception as e:
LOG.exception(e)
traceback.print_exc(file=sys.stdout)
pass
JarbasAl marked this conversation as resolved.
Show resolved Hide resolved

for client in client_connections:
client.write_message(message)
Expand Down
Loading