Skip to content

feat: Add storage events integration #130

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

Merged
merged 32 commits into from
Jul 17, 2025

Conversation

Matovidlo
Copy link
Contributor

  • Extend RawClient API to contain storage event.
  • Create mcp_context for tools execution, fill tool name and arguments of tools that were executed.
  • Extend tests to check send storage events.

@Matovidlo Matovidlo force-pushed the feat-add-storage-events-integration branch from e6cf0f6 to d04ad21 Compare July 1, 2025 08:34
@Matovidlo Matovidlo force-pushed the feat-add-storage-events-integration branch 2 times, most recently from 3024518 to 3b0f99a Compare July 3, 2025 06:48
@Matovidlo Matovidlo requested a review from Copilot July 3, 2025 08:24
Copilot

This comment was marked as outdated.

@Matovidlo Matovidlo force-pushed the feat-add-storage-events-integration branch from 763ba04 to fb0e11a Compare July 3, 2025 09:50
@Matovidlo Matovidlo marked this pull request as ready for review July 3, 2025 11:00
@Matovidlo Matovidlo requested review from ncapek and vita-stejskal July 3, 2025 11:00
@Matovidlo
Copy link
Contributor Author

@vita-stejskal https://keboola.atlassian.net/browse/AI-1192 this is not done in this PR I am unsure whether we need that. I think we are able to construct whole session from start to end (Except logginging in). I think that's I think part of Data dog as currently our events are binded with Storage API and are not for general use

@Matovidlo Matovidlo force-pushed the feat-add-storage-events-integration branch 2 times, most recently from ccb9e89 to 22579de Compare July 7, 2025 10:51
Copilot

This comment was marked as outdated.

@Matovidlo Matovidlo force-pushed the feat-add-storage-events-integration branch from 22579de to 066ffdd Compare July 7, 2025 11:08
@Matovidlo Matovidlo force-pushed the feat-add-storage-events-integration branch 2 times, most recently from 498ec8a to 111f370 Compare July 15, 2025 06:43
@Matovidlo Matovidlo force-pushed the feat-add-storage-events-integration branch from 111f370 to ce0c750 Compare July 15, 2025 06:53
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR integrates storage event logging by extending the RawKeboolaClient with a new trigger_event method and associated TriggerEventRequest model, enhances the tool_errors decorator to gather execution context and send events on errors, and updates unit and integration tests to verify event behavior and suppression.

  • Introduce trigger_event in RawKeboolaClient and the TriggerEventRequest Pydantic model
  • Extend tool_errors decorator to extract session/tool context and dispatch storage events on failures
  • Update tests to cover successful and error event payloads and ensure no events are sent on regular HTTP calls

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/test_client_events.py Add tests for trigger_event payloads and suppression on HTTP calls
src/keboola_mcp_server/tools/components/tools.py Switch to UTC‐aware timestamps with datetime.now(timezone.utc)
src/keboola_mcp_server/errors.py Implement context extraction helpers and enhance tool_errors decorator
src/keboola_mcp_server/client.py Add trigger_event method and TriggerEventRequest model
integtests/test_errors.py Add integration tests to verify event dispatch in tool_errors
integtests/conftest.py Introduce unique_id fixture for test resource IDs
Comments suppressed due to low confidence (3)

tests/test_client_events.py:125

  • Consider explicitly mocking trigger_event on the client and asserting mock_trigger_event.assert_not_called() to ensure no events are sent during regular HTTP calls.
        # No event should be triggered by HTTP methods anymore

src/keboola_mcp_server/client.py:267

  • The trigger_event method references os.environ but os is not imported in this file. Add import os at the top to avoid a NameError.
                    'appEnv': os.environ.get('APP_ENV', 'development'),

src/keboola_mcp_server/errors.py:218

  • The TriggerEventRequest model expects configuration_id (or alias configurationId), but here you're passing config_id. Rename this keyword to configuration_id to match the model field.
                                config_id=event_ctx.additional_context.get('config_id'),

@vita-stejskal vita-stejskal self-assigned this Jul 15, 2025
@mariankrotil mariankrotil merged commit 5cdb0e8 into main Jul 17, 2025
19 checks passed
@mariankrotil mariankrotil deleted the feat-add-storage-events-integration branch July 17, 2025 11:23
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