-
Notifications
You must be signed in to change notification settings - Fork 60
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
[Core] Replace pyjq
with jq.py
#1049
Conversation
Oh this may finally solve the problems I have always had installing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
port_ocean/tests/core/handlers/entity_processor/test_jq_entity_processor.py
Outdated
Show resolved
Hide resolved
port_ocean/tests/core/handlers/entity_processor/test_jq_entity_processor.py
Outdated
Show resolved
Hide resolved
value = await loop.run_in_executor(None, func.first) | ||
execute_time = loop.time() - start_time - compile_time | ||
logger.debug( | ||
f"Search for pattern {execute_time:.2f} seconds, compile time {compile_time:.2f} seconds" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is golden, I wonder if it's possible to also send the explicit metrics as values in addition to the debug log text
@pytest.mark.asyncio | ||
class TestJQEntityProcessor: | ||
|
||
@pytest.fixture |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh nice, closure automagically since it's in the test class?
If so, good practice that's worth sharing.
@@ -78,6 +78,7 @@ types-python-dateutil = "^2.9.0.20240316" | |||
types-pyyaml = "^6.0.12.10" | |||
types-toml = "^0.10.8.6" | |||
yamllint = "^1.32.0" | |||
pytest-timeout = "^2.3.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be worth aligning this in all integrations + cookie cutter as well
@@ -174,4 +175,4 @@ exclude = ''' | |||
[tool.pytest.ini_options] | |||
asyncio_mode = "auto" | |||
asyncio_default_fixture_loop_scope = "function" | |||
addopts = "-vv -n auto --ignore-glob='./integrations/*' ./port_ocean/tests" | |||
addopts = "-vv -n auto --durations=10 --color=yes --ignore-glob='./integrations/*' ./port_ocean/tests" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto re integrations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Description
What -
pyjq
withjq.py
IN
filter not supported by ocean JQ processor #1001Why - Upgrades internal jq version to latest 1.7.1 from 1.5.1
How -
Type of change
Please leave one option from the following and delete the rest:
All tests should be run against the port production environment(using a testing org).
Core testing checklist
Integration testing checklist
examples
folder in the integration directory.Preflight checklist
Screenshots
Include screenshots from your environment showing how the resources of the integration will look.
API Documentation
Provide links to the API documentation used for this integration.