Fix plugin regressions #688

cosmicBboy · 2021-10-06T21:30:26Z

Signed-off-by: Niels Bantilan [email protected]

TL;DR

A few regressions in the flytekit plugins accumulated because the plugins CI used a pinned version of flytekit. This PR fixes the regressions and updates pythonbuild.yml to use a developer installation of flytekit to test whether flytekit changes break the plugins

Type

Bug Fix
Feature
Plugin

Are all requirements met?

Complete description

There was a regression in several flytekit plugins, one of which was caused by this PR: #655. The fixes were:

implement assert_type in pandera type transformer
add sqlalchemy to greatexpectations setup.py
update a few imports that broke due to changes in plugin directory structure

Tracking Issue

https://github.com/lyft/flyte/issues/

Follow-up issue

NA
OR
https://github.com/lyft/flyte/issues/

Signed-off-by: Niels Bantilan <[email protected]>

codecov · 2021-10-06T21:55:29Z

Codecov Report

Merging #688 (f7d5032) into master (652fd22) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #688   +/-   ##
=======================================
  Coverage   85.68%   85.68%           
=======================================
  Files         355      355           
  Lines       29640    29684   +44     
  Branches     2411     2426   +15     
=======================================
+ Hits        25396    25436   +40     
- Misses       3604     3606    +2     
- Partials      640      642    +2

Impacted Files	Coverage Δ
flytekit/remote/remote.py	`72.46% <0.00%> (-0.79%)`	⬇️
tests/flytekit/unit/core/test_type_engine.py	`99.71% <0.00%> (+0.01%)`	⬆️
flytekit/core/type_engine.py	`88.38% <0.00%> (+0.57%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 652fd22...f7d5032. Read the comment docs.

Signed-off-by: Niels Bantilan <[email protected]>

eapolinario · 2021-10-07T00:35:35Z

plugins/flytekit-pandera/flytekitplugins/pandera/schema.py

@@ -53,6 +55,11 @@ def _get_schema_type(self, t: Type[pandera.typing.DataFrame]) -> SchemaType:
    def get_literal_type(self, t: Type[pandera.typing.DataFrame]) -> LiteralType:
        return LiteralType(schema=self._get_schema_type(t))

+    def assert_type(self, t: Type[T], v: T):
+        if not hasattr(t, "__origin__") and not isinstance(v, t):
+        # if not hasattr(t, "__origin__") and not isinstance(v, (t, pandas.DataFrame)):


remove commented code.

this isn't ready for review yet 😅

oops, my bad. Can you ping when it's ready for review?

kumare3 · 2021-10-07T03:50:08Z

@cosmicBboy thank you for the catch this is my bad!

Signed-off-by: Niels Bantilan <[email protected]>

eapolinario

Can you update the description and mention that this PR actually fixes all plugin regressions not only pandera?

* fix pandera regression Signed-off-by: Niels Bantilan <[email protected]> * install plugin with pip Signed-off-by: Niels Bantilan <[email protected]> * fix pandera plugin tests Signed-off-by: Niels Bantilan <[email protected]> * wip Signed-off-by: Niels Bantilan <[email protected]> * wip Signed-off-by: Niels Bantilan <[email protected]> * wip Signed-off-by: Niels Bantilan <[email protected]> * wip Signed-off-by: Niels Bantilan <[email protected]> * wip Signed-off-by: Niels Bantilan <[email protected]> * wip Signed-off-by: Niels Bantilan <[email protected]> * add spark flytekit plugin to papermill test_requires Signed-off-by: Niels Bantilan <[email protected]> * wip Signed-off-by: Niels Bantilan <[email protected]> * wip Signed-off-by: Niels Bantilan <[email protected]> * wip Signed-off-by: Niels Bantilan <[email protected]> * wip Signed-off-by: Niels Bantilan <[email protected]> * add sqlalchemy to great expectations plugin Signed-off-by: Niels Bantilan <[email protected]> * wip Signed-off-by: Niels Bantilan <[email protected]> * wip Signed-off-by: Niels Bantilan <[email protected]> * plugins plugins plugins! Signed-off-by: Niels Bantilan <[email protected]> * lint Signed-off-by: Niels Bantilan <[email protected]>

* Run 3.9 in CI Signed-off-by: Eduardo Apolinario <[email protected]> * Also run 3.9 in plugins tests. Signed-off-by: Eduardo Apolinario <[email protected]> * Fix tests Signed-off-by: Eduardo Apolinario <[email protected]> * Account for the different exception type raised in case of wrong types Signed-off-by: Eduardo Apolinario <[email protected]> * Exclude spark2 Signed-off-by: Eduardo Apolinario <[email protected]> * Failed to load json_data to dataclass (#684) Signed-off-by: Kevin Su <[email protected]> Signed-off-by: Eduardo Apolinario <[email protected]> * Remove sync singledispatch, add option for top-level only sync (#681) Signed-off-by: Yee Hing Tong <[email protected]> Signed-off-by: Eduardo Apolinario <[email protected]> * Failed to transform path string to Literal (#689) Signed-off-by: Kevin Su <[email protected]> Signed-off-by: Eduardo Apolinario <[email protected]> * Don't node sync on remote wait (#690) * no node sync Signed-off-by: Yee Hing Tong <[email protected]> * add param to wait Signed-off-by: Yee Hing Tong <[email protected]> Signed-off-by: Eduardo Apolinario <[email protected]> * Fix plugin regressions (#688) * fix pandera regression Signed-off-by: Niels Bantilan <[email protected]> * install plugin with pip Signed-off-by: Niels Bantilan <[email protected]> * fix pandera plugin tests Signed-off-by: Niels Bantilan <[email protected]> * wip Signed-off-by: Niels Bantilan <[email protected]> * wip Signed-off-by: Niels Bantilan <[email protected]> * wip Signed-off-by: Niels Bantilan <[email protected]> * wip Signed-off-by: Niels Bantilan <[email protected]> * wip Signed-off-by: Niels Bantilan <[email protected]> * wip Signed-off-by: Niels Bantilan <[email protected]> * add spark flytekit plugin to papermill test_requires Signed-off-by: Niels Bantilan <[email protected]> * wip Signed-off-by: Niels Bantilan <[email protected]> * wip Signed-off-by: Niels Bantilan <[email protected]> * wip Signed-off-by: Niels Bantilan <[email protected]> * wip Signed-off-by: Niels Bantilan <[email protected]> * add sqlalchemy to great expectations plugin Signed-off-by: Niels Bantilan <[email protected]> * wip Signed-off-by: Niels Bantilan <[email protected]> * wip Signed-off-by: Niels Bantilan <[email protected]> * plugins plugins plugins! Signed-off-by: Niels Bantilan <[email protected]> * lint Signed-off-by: Niels Bantilan <[email protected]> * Exclude does not understand lists Signed-off-by: Eduardo Apolinario <[email protected]> * Invert python version check Signed-off-by: Eduardo Apolinario <[email protected]> * Invert python version check for real this time Signed-off-by: Eduardo Apolinario <[email protected]> * Enable 3.10 just for kicks Signed-off-by: Eduardo Apolinario <[email protected]> * Add quotes around python versions Yes, this is needed, please see https://dev.to/hugovk/the-python-3-1-problem-85g. Signed-off-by: Eduardo Apolinario <[email protected]> * Revert "Add quotes around python versions" This reverts commit 4d619d5. Signed-off-by: Eduardo Apolinario <[email protected]> * Revert "Enable 3.10 just for kicks" This reverts commit bd6d694. Signed-off-by: Eduardo Apolinario <[email protected]> * wip - restricted types Signed-off-by: Eduardo Apolinario <[email protected]> * wip - restricted types Signed-off-by: Eduardo Apolinario <[email protected]> * Comment use of restricted types in get_transformer Signed-off-by: Eduardo Apolinario <[email protected]> * Publish 3.9 image Signed-off-by: Eduardo Apolinario <[email protected]> * Add python 3.9 to the list of supported languages Signed-off-by: Eduardo Apolinario <[email protected]> * Comment RestrictedTypeTransformer and add one test case. Signed-off-by: Eduardo Apolinario <[email protected]> * Review feedback Signed-off-by: Eduardo Apolinario <[email protected]> * Comment RestrictedTypeTransformer Signed-off-by: Eduardo Apolinario <[email protected]> * Handle the TypeError Signed-off-by: Eduardo Apolinario <[email protected]> * Remove breakpoint Signed-off-by: Eduardo Apolinario <[email protected]> Co-authored-by: Eduardo Apolinario <[email protected]> Co-authored-by: Kevin Su <[email protected]> Co-authored-by: Yee Hing Tong <[email protected]> Co-authored-by: Niels Bantilan <[email protected]>

fix pandera regression

8bb56a8

Signed-off-by: Niels Bantilan <[email protected]>

cosmicBboy requested review from katrogan, kumare3 and wild-endeavor as code owners October 6, 2021 21:30

install plugin with pip

8237bab

Signed-off-by: Niels Bantilan <[email protected]>

cosmicBboy added 6 commits October 6, 2021 18:05

fix pandera plugin tests

53566f7

Signed-off-by: Niels Bantilan <[email protected]>

wip

7d4c7df

Signed-off-by: Niels Bantilan <[email protected]>

wip

007b5a9

Signed-off-by: Niels Bantilan <[email protected]>

wip

cef80a5

Signed-off-by: Niels Bantilan <[email protected]>

wip

abbbeb1

Signed-off-by: Niels Bantilan <[email protected]>

wip

ef58ed5

Signed-off-by: Niels Bantilan <[email protected]>

wild-endeavor changed the title ~~fix pandera regression~~ Fix plugin regressions Oct 6, 2021

eapolinario reviewed Oct 7, 2021

View reviewed changes

cosmicBboy added 11 commits October 7, 2021 11:02

wip

01b69fc

Signed-off-by: Niels Bantilan <[email protected]>

add spark flytekit plugin to papermill test_requires

18c66c2

Signed-off-by: Niels Bantilan <[email protected]>

wip

62fc252

Signed-off-by: Niels Bantilan <[email protected]>

wip

5b48c94

Signed-off-by: Niels Bantilan <[email protected]>

wip

8f66a44

Signed-off-by: Niels Bantilan <[email protected]>

wip

f90a58a

Signed-off-by: Niels Bantilan <[email protected]>

add sqlalchemy to great expectations plugin

8247279

Signed-off-by: Niels Bantilan <[email protected]>

wip

b0016b7

Signed-off-by: Niels Bantilan <[email protected]>

wip

1e1f482

Signed-off-by: Niels Bantilan <[email protected]>

plugins plugins plugins!

db4c9e5

Signed-off-by: Niels Bantilan <[email protected]>

lint

f7d5032

Signed-off-by: Niels Bantilan <[email protected]>

eapolinario approved these changes Oct 7, 2021

View reviewed changes

cosmicBboy merged commit 6c03203 into master Oct 7, 2021

cosmicBboy deleted the fix-pandera branch October 7, 2021 21:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix plugin regressions #688

Fix plugin regressions #688

cosmicBboy commented Oct 6, 2021 •

edited

Loading

codecov bot commented Oct 6, 2021 •

edited

Loading

eapolinario Oct 7, 2021

cosmicBboy Oct 7, 2021

eapolinario Oct 7, 2021

kumare3 commented Oct 7, 2021

eapolinario left a comment

Fix plugin regressions #688

Fix plugin regressions #688

Conversation

cosmicBboy commented Oct 6, 2021 • edited Loading

TL;DR

Type

Are all requirements met?

Complete description

Tracking Issue

Follow-up issue

codecov bot commented Oct 6, 2021 • edited Loading

Codecov Report

eapolinario Oct 7, 2021

Choose a reason for hiding this comment

cosmicBboy Oct 7, 2021

Choose a reason for hiding this comment

eapolinario Oct 7, 2021

Choose a reason for hiding this comment

kumare3 commented Oct 7, 2021

eapolinario left a comment

Choose a reason for hiding this comment

cosmicBboy commented Oct 6, 2021 •

edited

Loading

codecov bot commented Oct 6, 2021 •

edited

Loading