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

Integrating the notifications to Zoom Webhook #83

Merged
merged 2 commits into from
Jun 24, 2024

Conversation

amaldevk88
Copy link
Contributor

Description

I've expanded the notification capability within the spark_expectations library to include Zoom alongside the existing Slack and Team notifications. This improvement allows users to receive updates on data quality job execution status across Zoom,Slack and Teams platforms.

Related Issue

Please link to the issue here: #82

Motivation and Context

The motivation behind this change is to broaden the reach of data quality framework execution status notifications. While Slack is a popular communication platform, some organizations prefer or exclusively use Zoom. By incorporating Zoom notifications into the spark_expectation library, we enhance its usability for a wider audience, ensuring that data quality job execution status alerts are accessible across multiple collaboration tools.

How Has This Been Tested?

To ensure the reliability and functionality of the Zoom notification feature, I thoroughly tested the following aspects:
Unit Tests:- Implemented unit tests to verify the correctness of the Zoom notification implementation.
Integration Testing: Conducted integration tests by simulating different data quality scenarios and validating the generation and delivery of notifications to Teams.
Environment: Tested the feature in various environments like databricks, local IDEs as well as to ensure compatibility and functionality across different setups.
Message Formatting: Checked the appearance and readability of messages sent to Zoom to ensure they display correctly.
This comprehensive testing approach aimed to validate the feature's effectiveness, compatibility, and correctness before integrating it into the spark_expectations library.

Screenshots (if appropriate):

Link to Unit test document:-
Test.Results.Proofs.-.SE.Zoom.Notif.Feature.zip

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@asingamaneni
Copy link
Collaborator

@amaldevk88 Looks like the lint checks failed, can you please correct and run make cov in your local to validate everything is right. Thanks!

@amaldevk88
Copy link
Contributor Author

@amaldevk88 Looks like the lint checks failed, can you please correct and run make cov in your local to validate everything is right. Thanks!

lint check issues are fixed.

@asingamaneni
Copy link
Collaborator

@amaldevk88 can you please fix the unittests.

@amaldevk88
Copy link
Contributor Author

regex pattern mismatch issues in Unit test are fixed. Kindly rerun unit test.

@amaldevk88
Copy link
Contributor Author

just rebased code and pushed again. Kindly rerun

poetry.lock Outdated Show resolved Hide resolved
@amaldevk88 amaldevk88 reopened this May 30, 2024
@amaldevk88
Copy link
Contributor Author

Please review and approve the merge

Copy link
Collaborator

@asingamaneni asingamaneni left a comment

Choose a reason for hiding this comment

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

LGTM

@asingamaneni asingamaneni merged commit d3edcde into Nike-Inc:main Jun 24, 2024
4 checks passed
cskcvarma pushed a commit to cskcvarma/spark-expectations that referenced this pull request Jul 22, 2024
* Update CONTRIBUTORS.md

* Added zoom notification
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