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

fix the bug for Final table write happens before running final_agg_dq and final_query_dq expectations #22

Merged
merged 3 commits into from
Sep 6, 2023

Conversation

phanikumarvemuri
Copy link
Contributor

@phanikumarvemuri phanikumarvemuri commented Sep 4, 2023

This PR addresses the issue - #11

Description

when write_to_table option is set, the write should only happen when all dq checks are completed. Right now the table is being saved after running row_dq rules but agg_final or query_final may fail . So moving the write part to expectations.py

Related Issue

#11

Motivation and Context

If the agg_final and query_final dq rule fails, there is no point in saving the dataframe as table.

How Has This Been Tested?

Ran locally and unit tested

Screenshots (if appropriate):

Screenshot 2023-09-03 at 7 14 56 AM

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.

@codecov
Copy link

codecov bot commented Sep 5, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (f5b3ea5) 100.00% compared to head (7c2e098) 100.00%.
Report is 1 commits behind head on main.

❗ Current head 7c2e098 differs from pull request most recent head d73ac47. Consider uploading reports for the commit d73ac47 to get more accurate results

Additional details and impacted files
@@            Coverage Diff            @@
##              main       #22   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           22        22           
  Lines         1369      1369           
=========================================
  Hits          1369      1369           
Files Changed Coverage Δ
spark_expectations/utils/regulate_flow.py 100.00% <ø> (ø)
spark_expectations/core/expectations.py 100.00% <100.00%> (ø)

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

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, thanks!

@Umeshsp22
Copy link
Collaborator

LGTM

@Umeshsp22 Umeshsp22 closed this Sep 6, 2023
@Umeshsp22 Umeshsp22 reopened this Sep 6, 2023
@asingamaneni asingamaneni merged commit 61243d8 into Nike-Inc:main Sep 6, 2023
asingamaneni added a commit that referenced this pull request Jul 29, 2024
* Merge pull request #1

* Base setup for react ui

* Merge pull request #2

* Zustand + React Query setup

* Integrating Zustand, RQ and api.github with a basic implementation

* Completed the mantine + dev tooling setup and implemented a layout  (#3)

* Local test, vite, linting and prettier setup.
Switching to mantine instead of material ui

* Updating readme.md with run scripts details

* Adding basic app layout with mantine

* Adding auth provider which shows a modal for users to enter their github token (#4)

* GitHub Integration + Render Repos List And User Card  (#5)

* 1. Segregating header into a new component
2. Updating header with new styles

* Implement a ReposList component with dropdown.
Supports smooth scrolling and autocomplete search
ReposList shows in Header component.

* Implemented a UserButton Component.
UserButton is in Header

* Implemented use repos
Integrated it with ReposList

Implemented useUser
Integrated it with UserButton

* Update Readme.md

* Reverting IDE changes

* 1. Improved token handling
2. Improved api-client
3. Added a Loading component

* Linting fixes

* Failed attempt at wrapping Error and Loading using a HOC. Will investigate this later.

* Updates to Header and UserMenu

* Add a sample NavBar

* updating tests

* Implement Base Structure and GitHub OAuth Login (#97)

* Initializing theme files (#11)

* Adding a basic mantine theme file. It will be updated as the code base develops.

Any colors or gaps will not be hardcoded.

They should first be defined in the theme and then should be used in the code.

* updating .gitignore

* Includes a comprehensive test setup with mock data and support structure. (#12)

setting up tests in the future will be easier.

* 9 tech debt setup public private routing (#13)

* Working version of routing

* Implemented routing, however tests for the login page needs to figured out.

* implemented notification from mantine (#14)

* 8 feature implement login functionality using GitHub oauth (#15)

* clean up

* A simple but beautiful login page

* Github Oauth setup

* 16 tech debt complete todos (#17)

* Fixed testing setup and have all tests for OAuth.

* Fixed remaining TODOs, added tests.

* Adding a DEVELOPER.md and .env.example

* Initializing theme files (#11)

* Adding a basic mantine theme file. It will be updated as the code base develops.

Any colors or gaps will not be hardcoded.

They should first be defined in the theme and then should be used in the code.

* updating .gitignore

* Includes a comprehensive test setup with mock data and support structure. (#12)

setting up tests in the future will be easier.

* 9 tech debt setup public private routing (#13)

* Working version of routing

* Implemented routing, however tests for the login page needs to figured out.

* implemented notification from mantine (#14)

* 8 feature implement login functionality using GitHub oauth (#15)

* clean up

* A simple but beautiful login page

* Github Oauth setup

* 16 tech debt complete todos (#17)

* Fixed testing setup and have all tests for OAuth.

* Fixed remaining TODOs, added tests.

* Adding a DEVELOPER.md and .env.example

* Restructuring + layout (#19)

* adding yaml file

* 20 load list of organizations on the left cell (#21)

* WIP

* fetch user name before rendering main page

* implement api for user repos

* show available repos on the left hand side

* Rendering an un nested list of files

* rendering yaml files in each project

* implemented rules table

* editing rules.yaml

* Build a working version (#22)

* working modal

* working flow

* Updated repos list

* Refactored CommitsList.tsx and apis

* minor fixes

* clean up

---------

Co-authored-by: Ashok Singamaneni <ashok.singamaneni@nike.com>
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.

3 participants