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.

Sign up for GitHub

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

Jump to bottom

Add back attempt to use existing serialization settings when running #1529

Merged
merged 2 commits into from
Mar 1, 2023
Merged

Add back attempt to use existing serialization settings when running #1529

merged 2 commits into from
Mar 1, 2023

Conversation

wild-endeavor
Copy link
Contributor

@wild-endeavor wild-endeavor commented Mar 1, 2023 •
edited
Loading

TL;DR

This PR is in response to flyteorg/flyte#3169 which was incorrectly patched in #1378 and later reverted in #1460 because of
flyteorg/flyte#3324

This is still not the complete fix. For that we need to enable flytectl to update the serialized context. (which means we'll probably make serialization settings a protobuf object.)

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

This was tested with the test case in flyteorg/flyte#3324 and the failure case repro'ed with this branch.

Tracking Issue

flyteorg/flyte#3169

@wild-endeavor
revert the revert
78d8ca2 
Signed-off-by: Yee Hing Tong <wild-endeavor@users.noreply.github.com>
@wild-endeavor
nit
ffe7211 
Signed-off-by: Yee Hing Tong <wild-endeavor@users.noreply.github.com>
@wild-endeavor wild-endeavor changed the title Re Add back attempt to use existing serialization settings when running Mar 1, 2023
@codecov
Copy link

codecov bot commented Mar 1, 2023 •
edited
Loading

Codecov Report

Merging #1529 (ffe7211) into master (d0b72a8) will increase coverage by 0.17%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1529      +/-   ##
==========================================
+ Coverage   69.09%   69.27%   +0.17%     
==========================================
  Files         315      315              
  Lines       28812    28853      +41     
  Branches     2333     2742     +409     
==========================================
+ Hits        19907    19987      +80     
+ Misses       8383     8349      -34     
+ Partials      522      517       -5
Impacted Files Coverage Δ
tests/flytekit/unit/bin/test_python_entrypoint.py 99.55% <100.00%> (+0.47%) ⬆️
flytekit/interfaces/random.py 20.00% <0.00%> (-5.00%) ⬇️
flytekit/configuration/internal.py 16.21% <0.00%> (-1.97%) ⬇️
flytekit/types/directory/types.py 55.73% <0.00%> (-0.47%) ⬇️
flytekit/types/file/file.py 60.68% <0.00%> (-0.43%) ⬇️
flytekit/clients/helpers.py 5.88% <0.00%> (-0.37%) ⬇️
flytekit/models/security.py 12.82% <0.00%> (-0.34%) ⬇️
flytekit/clis/flyte_cli/main.py 44.34% <0.00%> (-0.29%) ⬇️
flytekit/types/schema/types.py 38.15% <0.00%> (-0.16%) ⬇️
...t/clients/grpc_utils/wrap_exception_interceptor.py 2.70% <0.00%> (-0.08%) ⬇️
... and 10 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@wild-endeavor wild-endeavor mentioned this pull request Mar 1, 2023
Closed
2 tasks
@wild-endeavor wild-endeavor merged commit 6b56fb5 into master Mar 1, 2023
wild-endeavor added a commit that referenced this pull request Mar 7, 2023
@wild-endeavor
Add back attempt to use existing serialization settings when running (#…
af998b7 
…1529)

Signed-off-by: Yee Hing Tong <wild-endeavor@users.noreply.github.com>
wild-endeavor added a commit that referenced this pull request Mar 7, 2023
@wild-endeavor
Add back attempt to use existing serialization settings when running (#…
2d808ca 
…1529)

Signed-off-by: Yee Hing Tong <wild-endeavor@users.noreply.github.com>
eapolinario added a commit that referenced this pull request Mar 8, 2023
@wild-endeavor @eapolinario
@pingsutw @samhita-alla @cosmicBboy
Backport 1.4 onto v1.2 release branch (#1542)
074262b 
* Create non-root user after apt-get (#1519)

* Create non-root user after apt-get

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Create user after pip install

Signed-off-by: Kevin Su <pingsutw@apache.org>

---------

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
Signed-off-by: Kevin Su <pingsutw@apache.org>
Co-authored-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
Co-authored-by: Kevin Su <pingsutw@apache.org>

* Add root pyflyte reference to docs (#1520)

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
Co-authored-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* DuckDB plugin (#1419)

* DuckDB integration

Signed-off-by: Samhita Alla <aallasamhita@gmail.com>

* add sd test and fix import

Signed-off-by: Samhita Alla <samhitaalla@Samhitas-MacBook-Pro.local>
Signed-off-by: Samhita Alla <aallasamhita@gmail.com>

* fix lint error

Signed-off-by: Samhita Alla <aallasamhita@gmail.com>

* fix lint error

Signed-off-by: Samhita Alla <aallasamhita@gmail.com>

* list to List

Signed-off-by: Samhita Alla <aallasamhita@gmail.com>

* lint

Signed-off-by: Samhita Alla <aallasamhita@gmail.com>

* incorporated suggestions

Signed-off-by: Samhita Alla <aallasamhita@gmail.com>

* add duckdb to requirements and add gh action to detect doc warnings and errors

Signed-off-by: Samhita Alla <aallasamhita@gmail.com>

* gh action: python 3.9

Signed-off-by: Samhita Alla <aallasamhita@gmail.com>

* docs python 3.8 to 3.9

Signed-off-by: Samhita Alla <aallasamhita@gmail.com>

---------

Signed-off-by: Samhita Alla <aallasamhita@gmail.com>
Signed-off-by: Samhita Alla <samhitaalla@Samhitas-MacBook-Pro.local>
Co-authored-by: Kevin Su <pingsutw@apache.org>

* add string as a valid input (#1527)

* add string as a valid input

Signed-off-by: Samhita Alla <aallasamhita@gmail.com>

* isort

Signed-off-by: Samhita Alla <aallasamhita@gmail.com>

* tests

Signed-off-by: Samhita Alla <aallasamhita@gmail.com>

* Lint

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

---------

Signed-off-by: Samhita Alla <aallasamhita@gmail.com>
Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
Co-authored-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Add back attempt to use existing serialization settings when running (#1529)

Signed-off-by: Yee Hing Tong <wild-endeavor@users.noreply.github.com>

* update configuration docs, fix some docstrings (#1530)

* update configuration docs, fix some docstrings

Signed-off-by: Niels Bantilan <niels.bantilan@gmail.com>

* update copy

Signed-off-by: Niels Bantilan <niels.bantilan@gmail.com>

* add config init command

Signed-off-by: Niels Bantilan <niels.bantilan@gmail.com>

---------

Signed-off-by: Niels Bantilan <niels.bantilan@gmail.com>

* Revert "Make flytekit comply with PEP-561 (#1516)" (#1532)

This reverts commit b3ad158.

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
Co-authored-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
Signed-off-by: Yee Hing Tong <wild-endeavor@users.noreply.github.com>

* Failed to initialize FlyteInvalidInputException (#1534)

Signed-off-by: Kevin Su <pingsutw@apache.org>

* cherry pick pin fsspec commit

Signed-off-by: Yee Hing Tong <wild-endeavor@users.noreply.github.com>

* Set flytekit<1.3.0 in duckdb tests

Signed-off-by: eduardo apolinario <eapolinario@users.noreply.github.com>

* Fix flyteidl==1.2.9 in doc-requirements.txt

Signed-off-by: eduardo apolinario <eapolinario@users.noreply.github.com>

* No duckdb documentation

Signed-off-by: eduardo apolinario <eapolinario@users.noreply.github.com>

* Linting

Signed-off-by: eduardo apolinario <eapolinario@users.noreply.github.com>

---------

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Samhita Alla <aallasamhita@gmail.com>
Signed-off-by: Samhita Alla <samhitaalla@Samhitas-MacBook-Pro.local>
Signed-off-by: Yee Hing Tong <wild-endeavor@users.noreply.github.com>
Signed-off-by: Niels Bantilan <niels.bantilan@gmail.com>
Signed-off-by: eduardo apolinario <eapolinario@users.noreply.github.com>
Co-authored-by: Eduardo Apolinario <653394+eapolinario@users.noreply.github.com>
Co-authored-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
Co-authored-by: Kevin Su <pingsutw@apache.org>
Co-authored-by: Samhita Alla <aallasamhita@gmail.com>
Co-authored-by: Niels Bantilan <niels.bantilan@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eapolinario eapolinario eapolinario approved these changes

@kumare3 kumare3 Awaiting requested review from kumare3 kumare3 is a code owner

@pingsutw pingsutw Awaiting requested review from pingsutw pingsutw is a code owner

@cosmicBboy cosmicBboy Awaiting requested review from cosmicBboy cosmicBboy is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@wild-endeavor @eapolinario