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

Feature: Connectors workflow #140

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

Feature: Connectors workflow #140

wants to merge 14 commits into from

Conversation

vg-svitla
Copy link
Contributor

No description provided.

@vg-svitla vg-svitla requested a review from squioc October 3, 2024 17:26
Copy link

github-actions bot commented Oct 3, 2024

Test Results

219 tests   - 3   218 ✅  - 3   1m 44s ⏱️ ±0s
  1 suites ±0     1 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit 604ce4f. ± Comparison against base commit 7c6d61f.

This pull request removes 6 and adds 3 tests. Note that renamed tests count towards both.
tests.aio.helpers.http.test_http_client_session ‑ test_http_client_get_data
tests.aio.helpers.http.test_http_client_session ‑ test_http_client_get_data_async_limiter
tests.aio.helpers.http.test_http_token_refresher ‑ test_token_refresher_1
tests.aio.helpers.http.test_http_token_refresher ‑ test_token_refresher_2
tests.aio.helpers.http.test_http_token_refresher ‑ test_token_refresher_with_token
tests.aio.test_connector ‑ test_async_connector_rate_limiter
tests.aio.test_connector ‑ test_async_connector_async_next_run
tests.connectors.test_connector ‑ test_connector_next_run
tests.http.aio.examples.test_oauth_token_auth_client ‑ test_get_events_example_method

♻️ This comment has been updated with latest results.

Copy link

codecov bot commented Oct 4, 2024

Codecov Report

Attention: Patch coverage is 82.69231% with 27 lines in your changes missing coverage. Please review.

Project coverage is 90.21%. Comparing base (7c6d61f) to head (604ce4f).

Files with missing lines Patch % Lines
sekoia_automation/connector/__init__.py 81.48% 10 Missing ⚠️
sekoia_automation/connector/metrics.py 84.61% 8 Missing ⚠️
sekoia_automation/aio/connector.py 75.86% 7 Missing ⚠️
sekoia_automation/http/aio/token_refresher.py 66.66% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #140      +/-   ##
==========================================
- Coverage   90.76%   90.21%   -0.55%     
==========================================
  Files          35       35              
  Lines        2500     2566      +66     
==========================================
+ Hits         2269     2315      +46     
- Misses        231      251      +20     

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

Copy link
Collaborator

@squioc squioc left a comment

Choose a reason for hiding this comment

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

Thanks for the work. Some little changes to apply and some points to be aware of.

CHANGELOG.md Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
sekoia_automation/connector/__init__.py Show resolved Hide resolved
sekoia_automation/connector/__init__.py Outdated Show resolved Hide resolved
sekoia_automation/aio/connector.py Outdated Show resolved Hide resolved
# Conflicts:
#	CHANGELOG.md
#	pyproject.toml
#	sekoia_automation/aio/connector.py
#	sekoia_automation/aio/helpers/http/http_client.py
#	tests/aio/test_connector.py
@vg-svitla vg-svitla requested a review from squioc October 16, 2024 16:55
squioc
squioc previously approved these changes Oct 17, 2024
Copy link
Collaborator

@squioc squioc left a comment

Choose a reason for hiding this comment

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

LGTM

@vg-svitla vg-svitla requested a review from squioc October 25, 2024 11:39
@vg-svitla vg-svitla enabled auto-merge October 25, 2024 11:39
Copy link
Contributor

@lvoloshyn-sekoia lvoloshyn-sekoia left a comment

Choose a reason for hiding this comment

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

Thanks, that's a lot of work! Few questions:


class MetricsMixin:
_prometheus_namespace = "symphony_module_common"

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe we usually have an incoming messages counter too?

Suggested change
_incoming_events: Counter = Counter(
name="collected_messages",
documentation="Number of messages consumed",
namespace=prom_namespace,
labelnames=["intake_key"],
)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be honest I did not use it... But I can add if it is useful

sekoia_automation/connector/__init__.py Show resolved Hide resolved
@lvoloshyn-sekoia
Copy link
Contributor

@vg-svitla I've tried to use this version of sekoia-automation-sdk with existing connectors w/o any additional changes

poetry add git+https://github.com/SEKOIA-IO/sekoia-automation-sdk.git@fix/connector_workflow

But I got:

Traceback (most recent call last):
  File ".../automation-library/Office365/dev.py", line 12, in <module>
    from office365.management_api.connector import Office365Connector
  File ".../automation-library/Office365/office365/management_api/connector.py", line 13, in <module>
    from office365.metrics import FORWARD_EVENTS_DURATION, OUTCOMING_EVENTS
  File ".../automation-library/Office365/office365/metrics.py", line 6, in <module>
    OUTCOMING_EVENTS = Counter(
                       ^^^^^^^^
  File "/automation-library-N-S28PfP-py3.11/lib/python3.11/site-packages/prometheus_client/metrics.py", line 156, in __init__
    registry.register(self)
  File "/automation-library-N-S28PfP-py3.11/lib/python3.11/site-packages/prometheus_client/registry.py", line 43, in register
    raise ValueError(
ValueError: Duplicated timeseries in CollectorRegistry: {'symphony_module_common_forwarded_events_created', 'symphony_module_common_forwarded_events', 'symphony_module_common_forwarded_events_total'}

New metrics mechanism seems to be interfering with existing one.

…/connector_workflow

# Conflicts:
#	CHANGELOG.md
#	pyproject.toml
#	tests/aio/test_connector.py
Copy link
Contributor

@lvoloshyn-sekoia lvoloshyn-sekoia left a comment

Choose a reason for hiding this comment

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

LGTM

# Conflicts:
#	CHANGELOG.md
#	pyproject.toml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants