Skip to content

Push sample file to check PR#76

Open
Parkhi47 wants to merge 6 commits into
DataBytes-Organisation:feature/correlation-alert/parkhi-alertsfrom
Parkhi47:feature/correlation-alert/parkhi-alerts
Open

Push sample file to check PR#76
Parkhi47 wants to merge 6 commits into
DataBytes-Organisation:feature/correlation-alert/parkhi-alertsfrom
Parkhi47:feature/correlation-alert/parkhi-alerts

Conversation

@Parkhi47
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown
Collaborator

@senuradp senuradp left a comment

Choose a reason for hiding this comment

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

Good work fixing the alert-related folder structure and connecting threshold.py, alerts.py, and main.py into a runnable mock workflow. This is useful as an integration/demo contribution and helps validate how threshold logic and alert generation connect.

Before this can be merged into the final pipeline, a few alignment changes are needed:

  1. Please update the threshold ranges to match our final agreed system logic:
    No Alert: Δr < 0.3
    LOW: 0.3 ≤ Δr < 0.5
    MEDIUM: 0.5 ≤ Δr < 0.7
    HIGH: Δr ≥ 0.7

Currently HIGH starts at 0.8, so HIGH_THRESHOLD should be changed to 0.70.

  1. Please update the import in alerts.py. Since threshold.py is inside the correlation_alert module, avoid from threshold import get_alert_level. Use a module-safe import such as:
    from correlation_alert.threshold import get_alert_level
    or a relative import if appropriate.

  2. The current main.py uses hardcoded mock data, which is fine for testing/demo, but it should not replace the main reusable wrapper pipeline. For final integration, the alert logic should connect with the change results generated by the main correlation pipeline.

Overall, this is a useful integration fix, but it needs threshold and wrapper-format alignment before merging.

@senuradp
Copy link
Copy Markdown
Collaborator

Thanks for updating the implementation. I reviewed the updated main.py, and it is now cleaner and better structured as an alert-generation demo/integration helper.

The file correctly imports generate_alerts from correlation_alert.alerts, uses mock data only for demo/testing, and includes a run_alert_pipeline(data=None) function that can later accept real correlation-change output from the main pipeline.

One important note for final integration: this file currently focuses only on the alert side of the workflow. It does not replace the full correlation change alert wrapper, which still needs to handle preprocessing, rolling windows, correlation computation, change comparison, and alert generation together.

Overall, this is a useful alert pipeline/demo contribution and can be used during integration as a helper or testing runner.

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.

2 participants