From ae15c4e1b03f8669a2eaab6ef5db071017d1cb1a Mon Sep 17 00:00:00 2001 From: Eduardo Apolinario <653394+eapolinario@users.noreply.github.com> Date: Tue, 19 Oct 2021 14:17:36 -0700 Subject: [PATCH] Add official support for python 3.9 (#687) * Run 3.9 in CI Signed-off-by: Eduardo Apolinario * Also run 3.9 in plugins tests. Signed-off-by: Eduardo Apolinario * Fix tests Signed-off-by: Eduardo Apolinario * Account for the different exception type raised in case of wrong types Signed-off-by: Eduardo Apolinario * Exclude spark2 Signed-off-by: Eduardo Apolinario * Failed to load json_data to dataclass (#684) Signed-off-by: Kevin Su Signed-off-by: Eduardo Apolinario * Remove sync singledispatch, add option for top-level only sync (#681) Signed-off-by: Yee Hing Tong Signed-off-by: Eduardo Apolinario * Failed to transform path string to Literal (#689) Signed-off-by: Kevin Su Signed-off-by: Eduardo Apolinario * Don't node sync on remote wait (#690) * no node sync Signed-off-by: Yee Hing Tong * add param to wait Signed-off-by: Yee Hing Tong Signed-off-by: Eduardo Apolinario * Fix plugin regressions (#688) * fix pandera regression Signed-off-by: Niels Bantilan * install plugin with pip Signed-off-by: Niels Bantilan * fix pandera plugin tests Signed-off-by: Niels Bantilan * wip Signed-off-by: Niels Bantilan * wip Signed-off-by: Niels Bantilan * wip Signed-off-by: Niels Bantilan * wip Signed-off-by: Niels Bantilan * wip Signed-off-by: Niels Bantilan * wip Signed-off-by: Niels Bantilan * add spark flytekit plugin to papermill test_requires Signed-off-by: Niels Bantilan * wip Signed-off-by: Niels Bantilan * wip Signed-off-by: Niels Bantilan * wip Signed-off-by: Niels Bantilan * wip Signed-off-by: Niels Bantilan * add sqlalchemy to great expectations plugin Signed-off-by: Niels Bantilan * wip Signed-off-by: Niels Bantilan * wip Signed-off-by: Niels Bantilan * plugins plugins plugins! Signed-off-by: Niels Bantilan * lint Signed-off-by: Niels Bantilan * Exclude does not understand lists Signed-off-by: Eduardo Apolinario * Invert python version check Signed-off-by: Eduardo Apolinario * Invert python version check for real this time Signed-off-by: Eduardo Apolinario * Enable 3.10 just for kicks Signed-off-by: Eduardo Apolinario * 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 * Revert "Add quotes around python versions" This reverts commit 4d619d5c9e829aa1fdc18d2d87e1dbe4dbf42372. Signed-off-by: Eduardo Apolinario * Revert "Enable 3.10 just for kicks" This reverts commit bd6d69462cb722afc5e2eb09ce77e35da512d288. Signed-off-by: Eduardo Apolinario * wip - restricted types Signed-off-by: Eduardo Apolinario * wip - restricted types Signed-off-by: Eduardo Apolinario * Comment use of restricted types in get_transformer Signed-off-by: Eduardo Apolinario * Publish 3.9 image Signed-off-by: Eduardo Apolinario * Add python 3.9 to the list of supported languages Signed-off-by: Eduardo Apolinario * Comment RestrictedTypeTransformer and add one test case. Signed-off-by: Eduardo Apolinario * Review feedback Signed-off-by: Eduardo Apolinario * Comment RestrictedTypeTransformer Signed-off-by: Eduardo Apolinario * Handle the TypeError Signed-off-by: Eduardo Apolinario * Remove breakpoint Signed-off-by: Eduardo Apolinario Co-authored-by: Eduardo Apolinario Co-authored-by: Kevin Su Co-authored-by: Yee Hing Tong Co-authored-by: Niels Bantilan --- .github/workflows/pythonbuild.yml | 6 ++- .github/workflows/pythonpublish.yml | 14 ++++++ Dockerfile.py39 | 16 +++++++ flytekit/core/type_engine.py | 48 ++++++++++++++++----- setup.py | 1 + tests/flytekit/unit/core/test_type_hints.py | 10 ++++- 6 files changed, 81 insertions(+), 14 deletions(-) create mode 100644 Dockerfile.py39 diff --git a/.github/workflows/pythonbuild.yml b/.github/workflows/pythonbuild.yml index 057de08c66..9808851568 100644 --- a/.github/workflows/pythonbuild.yml +++ b/.github/workflows/pythonbuild.yml @@ -11,11 +11,13 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - python-version: [3.7, 3.8] + python-version: [3.7, 3.8, 3.9] spark-version-suffix: ["", "-spark2"] exclude: - python-version: 3.8 spark-version-suffix: "-spark2" + - python-version: 3.9 + spark-version-suffix: "-spark2" steps: - uses: actions/checkout@v2 - name: Set up Python ${{ matrix.python-version }} @@ -52,7 +54,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - python-version: [3.8] + python-version: [3.8, 3.9] plugin-names: - flytekit-aws-athena - flytekit-aws-sagemaker diff --git a/.github/workflows/pythonpublish.yml b/.github/workflows/pythonpublish.yml index 52ca29aedb..d203a75ff1 100644 --- a/.github/workflows/pythonpublish.yml +++ b/.github/workflows/pythonpublish.yml @@ -80,3 +80,17 @@ jobs: build_extra_args: "--compress=true --build-arg=VERSION=${{ steps.bump.outputs.version }} --build-arg=DOCKER_IMAGE=ghcr.io/flyteorg/flytekit:py38-${{ steps.bump.outputs.version }}" context: . dockerfile: Dockerfile.py38 + - name: Build & Push Flytekit Python3.9 Docker Image to Github Registry + uses: whoan/docker-build-with-cache-action@v5 + with: + # https://docs.github.com/en/packages/learn-github-packages/publishing-a-package + username: "${{ secrets.FLYTE_BOT_USERNAME }}" + password: "${{ secrets.FLYTE_BOT_PAT }}" + image_name: ${{ github.repository_owner }}/flytekit + image_tag: py39-latest,py39-${{ github.sha }},py39-${{ steps.bump.outputs.version }} + push_git_tag: true + push_image_and_stages: true + registry: ghcr.io + build_extra_args: "--compress=true --build-arg=VERSION=${{ steps.bump.outputs.version }} --build-arg=DOCKER_IMAGE=ghcr.io/flyteorg/flytekit:py39-${{ steps.bump.outputs.version }}" + context: . + dockerfile: Dockerfile.py39 diff --git a/Dockerfile.py39 b/Dockerfile.py39 new file mode 100644 index 0000000000..1a4d617aa0 --- /dev/null +++ b/Dockerfile.py39 @@ -0,0 +1,16 @@ +FROM python:3.9-slim-buster + +MAINTAINER Flyte Team +LABEL org.opencontainers.image.source https://github.com/flyteorg/flytekit + +RUN pip install awscli +RUN pip install gsutil + +ARG VERSION +ARG DOCKER_IMAGE + +RUN pip install -U flytekit==$VERSION + +WORKDIR /app + +ENV FLYTE_INTERNAL_IMAGE "$DOCKER_IMAGE" diff --git a/flytekit/core/type_engine.py b/flytekit/core/type_engine.py index 0187680965..39a7be762a 100644 --- a/flytekit/core/type_engine.py +++ b/flytekit/core/type_engine.py @@ -8,7 +8,7 @@ import mimetypes import typing from abc import ABC, abstractmethod -from typing import Optional, Type, cast +from typing import NamedTuple, Optional, Type, cast from dataclasses_json import DataClassJsonMixin, dataclass_json from google.protobuf import json_format as _json_format @@ -149,9 +149,10 @@ class RestrictedTypeError(Exception): pass -class RestrictedType(TypeTransformer[T], ABC): +class RestrictedTypeTransformer(TypeTransformer[T], ABC): """ - A Simple implementation of a type transformer that uses simple lambdas to transform and reduces boilerplate + Types registered with the RestrictedTypeTransformer are not allowed to be converted to and from literals. In other words, + Restricted types are not allowed to be used as inputs or outputs of tasks and workflows. """ def __init__(self, name: str, t: Type[T]): @@ -160,6 +161,12 @@ def __init__(self, name: str, t: Type[T]): def get_literal_type(self, t: Type[T] = None) -> LiteralType: raise RestrictedTypeError(f"Transformer for type {self.python_type} is restricted currently") + def to_literal(self, ctx: FlyteContext, python_val: T, python_type: Type[T], expected: LiteralType) -> Literal: + raise RestrictedTypeError(f"Transformer for type {self.python_type} is restricted currently") + + def to_python_value(self, ctx: FlyteContext, lv: Literal, expected_python_type: Type[T]) -> T: + raise RestrictedTypeError(f"Transformer for type {self.python_type} is restricted currently") + class DataclassTransformer(TypeTransformer[object]): """ @@ -339,10 +346,15 @@ class TypeEngine(typing.Generic[T]): """ _REGISTRY: typing.Dict[type, TypeTransformer[T]] = {} + _RESTRICTED_TYPES: typing.List[type] = [] _DATACLASS_TRANSFORMER: TypeTransformer = DataclassTransformer() @classmethod - def register(cls, transformer: TypeTransformer, additional_types: Optional[typing.List[Type]] = None): + def register( + cls, + transformer: TypeTransformer, + additional_types: Optional[typing.List[Type]] = None, + ): """ This should be used for all types that respond with the right type annotation when you use type(...) function """ @@ -356,6 +368,15 @@ def register(cls, transformer: TypeTransformer, additional_types: Optional[typin ) cls._REGISTRY[t] = transformer + @classmethod + def register_restricted_type( + cls, + name: str, + type: Type, + ): + cls._RESTRICTED_TYPES.append(type) + cls.register(RestrictedTypeTransformer(name, type)) + @classmethod def get_transformer(cls, python_type: Type) -> TypeTransformer[T]: """ @@ -399,10 +420,15 @@ def get_transformer(cls, python_type: Type) -> TypeTransformer[T]: for base_type in cls._REGISTRY.keys(): if base_type is None: continue # None is actually one of the keys, but isinstance/issubclass doesn't work on it - if isinstance(python_type, base_type) or ( - inspect.isclass(python_type) and issubclass(python_type, base_type) - ): - return cls._REGISTRY[base_type] + try: + if isinstance(python_type, base_type) or ( + inspect.isclass(python_type) and issubclass(python_type, base_type) + ): + return cls._REGISTRY[base_type] + except TypeError: + # As of python 3.9, calls to isinstance raise a TypeError if the base type is not a valid type, which + # is the case for one of the restricted types, namely NamedTuple. + logger.debug(f"Invalid base type {base_type} in call to isinstance", exc_info=True) raise ValueError(f"Type {python_type} not supported currently in Flytekit. Please register a new transformer") @classmethod @@ -896,9 +922,9 @@ def _register_default_type_transformers(): # that the return signature of a task can be a NamedTuple that contains another NamedTuple inside it. # Also, it's not entirely true that Flyte IDL doesn't support tuples. We can always fake them as structs, but we'll # hold off on doing that for now, as we may amend the IDL formally to support tuples. - TypeEngine.register(RestrictedType("non typed tuple", tuple)) - TypeEngine.register(RestrictedType("non typed tuple", typing.Tuple)) - TypeEngine.register(RestrictedType("named tuple", typing.NamedTuple)) + TypeEngine.register_restricted_type("non typed tuple", tuple) + TypeEngine.register_restricted_type("non typed tuple", typing.Tuple) + TypeEngine.register_restricted_type("named tuple", NamedTuple) _register_default_type_transformers() diff --git a/setup.py b/setup.py index 6fff1a7d8c..706b4dbbd8 100644 --- a/setup.py +++ b/setup.py @@ -110,6 +110,7 @@ "License :: OSI Approved :: Apache Software License", "Programming Language :: Python :: 3.7", "Programming Language :: Python :: 3.8", + "Programming Language :: Python :: 3.9", "Topic :: Scientific/Engineering", "Topic :: Scientific/Engineering :: Artificial Intelligence", "Topic :: Software Development", diff --git a/tests/flytekit/unit/core/test_type_hints.py b/tests/flytekit/unit/core/test_type_hints.py index 40bd8f2b73..c2957ad9e0 100644 --- a/tests/flytekit/unit/core/test_type_hints.py +++ b/tests/flytekit/unit/core/test_type_hints.py @@ -685,7 +685,15 @@ def my_wf(a: int, b: str) -> (int, str): assert context_manager.FlyteContextManager.size() == 1 -def test_cant_use_normal_tuples(): +def test_cant_use_normal_tuples_as_input(): + with pytest.raises(RestrictedTypeError): + + @task + def t1(a: tuple) -> str: + return a[0] + + +def test_cant_use_normal_tuples_as_output(): with pytest.raises(RestrictedTypeError): @task