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

Add http_proxy to client & Fix deviceflow #1611

Merged
merged 3 commits into from
May 16, 2023
Merged

Conversation

ByronHsu
Copy link
Collaborator

@ByronHsu ByronHsu commented Apr 27, 2023

TL;DR

  1. Add http_proxy to oauth2 client because in on-prem cluster use cases, the client might not have access to the external network
  2. Remove verification_url_complete because that field not exists in azure ad and the oauth2 standard.
  3. Concat the scopes by white space per the documentation

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

Tracking Issue

flyteorg/flyte#3620

@codecov
Copy link

codecov bot commented Apr 27, 2023

Codecov Report

Merging #1611 (bf022a4) into master (26d1f29) will increase coverage by 0.03%.
The diff coverage is 73.33%.

@@            Coverage Diff             @@
##           master    #1611      +/-   ##
==========================================
+ Coverage   71.22%   71.26%   +0.03%     
==========================================
  Files         334      334              
  Lines       30391    30477      +86     
  Branches     5490     5510      +20     
==========================================
+ Hits        21645    21718      +73     
- Misses       8206     8217      +11     
- Partials      540      542       +2     
Impacted Files Coverage Δ
flytekit/configuration/internal.py 16.00% <0.00%> (-0.22%) ⬇️
flytekit/configuration/__init__.py 37.38% <50.00%> (+0.07%) ⬆️
flytekit/clients/auth/token_client.py 52.50% <54.54%> (+1.18%) ⬆️
flytekit/clients/auth/authenticator.py 52.03% <83.33%> (+0.39%) ⬆️
flytekit/clients/auth_helper.py 45.20% <100.00%> (ø)
...s/flytekit/unit/clients/auth/test_authenticator.py 100.00% <100.00%> (ø)
...ts/flytekit/unit/clients/auth/test_token_client.py 100.00% <100.00%> (ø)

... and 4 files with indirect coverage changes

Copy link
Member

@pingsutw pingsutw left a comment

Choose a reason for hiding this comment

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

could you run make fmt & lint locally?

@ByronHsu ByronHsu mentioned this pull request May 9, 2023
3 tasks
print(
f"""
To Authenticate navigate in a browser to the following URL: {resp.verification_uri} and enter code: {resp.user_code}
OR copy paste the following URL: {resp.verification_uri_complete}
Copy link
Contributor

Choose a reason for hiding this comment

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

can we check for this field's existence, and show it if it's there? Even if it's not part of the standard, it's nice to have.

Copy link
Collaborator Author

@ByronHsu ByronHsu May 11, 2023

Choose a reason for hiding this comment

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

i don't think it should be there because uri_complete is not in standard. resp.verification_uri + resp.user_code is enough for login.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah but okta supports it. and lots of people use okta. it's not in the standard but i feel it's okay to support a nice to have.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@kumare3 wdyt ^^

Copy link
Contributor

Choose a reason for hiding this comment

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

can we just add back and merge?

Copy link
Contributor

@wild-endeavor wild-endeavor left a comment

Choose a reason for hiding this comment

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

two small changes otherwise +1

Copy link
Contributor

@wild-endeavor wild-endeavor left a comment

Choose a reason for hiding this comment

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

two small changes otherwise +1

ByronHsu added 2 commits May 10, 2023 22:10
Signed-off-by: byhsu <[email protected]>
@wild-endeavor
Copy link
Contributor

lint!

wild-endeavor
wild-endeavor previously approved these changes May 11, 2023
Copy link
Contributor

@wild-endeavor wild-endeavor left a comment

Choose a reason for hiding this comment

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

but needs lint

Signed-off-by: byhsu <[email protected]>
@ByronHsu ByronHsu merged commit c65397d into flyteorg:master May 16, 2023
eapolinario pushed a commit that referenced this pull request Jun 29, 2023
* Add http_proxy to client & Fix deviceflow

RB=3890720

Signed-off-by: byhsu <[email protected]>

* nit

Signed-off-by: byhsu <[email protected]>

* lint!

Signed-off-by: byhsu <[email protected]>

---------

Signed-off-by: byhsu <[email protected]>
Co-authored-by: byhsu <[email protected]>
eapolinario added a commit that referenced this pull request Jun 30, 2023
* FLYTECTL_CONFIG env var higher precedence, config flag respected in pyflyte package (#1662)

Signed-off-by: Yee Hing Tong <[email protected]>

* Change flytekit Pytorch, TFJob and MPI plugins to use new kubeflow config (#1627)

* upgrade tensorflow plugin to v1

Signed-off-by: Yubo Wang <[email protected]>

* minor fix

Signed-off-by: Yubo Wang <[email protected]>

* fix tests and lints

Signed-off-by: Yubo Wang <[email protected]>

* move models file into task make backward compatible

Signed-off-by: Yubo Wang <[email protected]>

Signed-off-by: Yubo Wang <[email protected]>

* add code example in README

Signed-off-by: Yubo Wang <[email protected]>

* bump flyteidl

Signed-off-by: Yubo Wang <[email protected]>

* add pytorch

Signed-off-by: Yubo Wang <[email protected]>

* add mpi and fix requirements.txt

Signed-off-by: Yubo Wang <[email protected]>

* lint and fmt

Signed-off-by: Yubo Wang <[email protected]>

* Regenerate requirements files using python 3.8

Signed-off-by: eduardo apolinario <[email protected]>

---------

Signed-off-by: Yubo Wang <[email protected]>
Signed-off-by: eduardo apolinario <[email protected]>
Co-authored-by: Yubo Wang <[email protected]>
Co-authored-by: eduardo apolinario <[email protected]>

* Root cert should be byte string when loading from caCertFilePath (#1669)

Signed-off-by: Yee Hing Tong <[email protected]>

* Explicitly set the content type for flyte deck (#1658)

* Set content type for flyte deck

Signed-off-by: Kevin Su <[email protected]>

* test

Signed-off-by: Kevin Su <[email protected]>

* test

Signed-off-by: Kevin Su <[email protected]>

* test

Signed-off-by: Kevin Su <[email protected]>

* test

Signed-off-by: Kevin Su <[email protected]>

* test

Signed-off-by: Kevin Su <[email protected]>

* test

Signed-off-by: Kevin Su <[email protected]>

* test

Signed-off-by: Kevin Su <[email protected]>

* test

Signed-off-by: Kevin Su <[email protected]>

* test

Signed-off-by: Kevin Su <[email protected]>

* test

Signed-off-by: Kevin Su <[email protected]>

* test

Signed-off-by: Kevin Su <[email protected]>

* test

Signed-off-by: Kevin Su <[email protected]>

* test

Signed-off-by: Kevin Su <[email protected]>

* nit

Signed-off-by: Kevin Su <[email protected]>

* test

Signed-off-by: Kevin Su <[email protected]>

* nit

Signed-off-by: Kevin Su <[email protected]>

* nit

Signed-off-by: Kevin Su <[email protected]>

---------

Signed-off-by: Kevin Su <[email protected]>

* Use protos of new kubeflow.pytorch plugin instead of legacy pytorch plugin (#1678)

Signed-off-by: Fabio Grätz <[email protected]>
Co-authored-by: Fabio Grätz <[email protected]>

* More time info for time line deck (#1680)

* more visualization

Signed-off-by: Yicheng-Lu-llll <[email protected]>

* more visualization

Signed-off-by: Yicheng-Lu-llll <[email protected]>

---------

Signed-off-by: Yicheng-Lu-llll <[email protected]>

* Add http_proxy to client & Fix deviceflow (#1611)

* Add http_proxy to client & Fix deviceflow

RB=3890720

Signed-off-by: byhsu <[email protected]>

* nit

Signed-off-by: byhsu <[email protected]>

* lint!

Signed-off-by: byhsu <[email protected]>

---------

Signed-off-by: byhsu <[email protected]>
Co-authored-by: byhsu <[email protected]>

* Pass verify flag to all authenticators (#1641)

Signed-off-by: byhsu <[email protected]>

* feat: Add Auth0/audience support for ClientCredentials flow (#1639)

* feat: Add Auth0/audience support for ClientCredentials flow

Signed-off-by: tnam <[email protected]>

* refactor: Remove unneeded variables & condense code

Signed-off-by: tnam <[email protected]>

* refactor: Reduce verbosity of code

Signed-off-by: tnam <[email protected]>

* refactor(chore): Remove unused commented code

Signed-off-by: tnam <[email protected]>

* fix: Missing comma in input args - authenticator.py 213

Signed-off-by: tnam <[email protected]>

* style: Run pre-commit against all files

Signed-off-by: tnam <[email protected]>

---------

Signed-off-by: tnam <[email protected]>
Co-authored-by: tnam <[email protected]>

* pyflyte run remote file (#1670)

Signed-off-by: ChungYujoyce <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Co-authored-by: Kevin Su <[email protected]>

* upload deck.html only with deck enable (#1693)

Signed-off-by: Yicheng-Lu-llll <[email protected]>

* Add dask plugin #patch (#1366)

* Add dummy task type to test backend plugin

Signed-off-by: Bernhard Stadlbauer <[email protected]>

* Add docs page

Signed-off-by: Bernhard Stadlbauer <[email protected]>

* Add dask models

Signed-off-by: Bernhard Stadlbauer <[email protected]>

* Add function to convert resources

Signed-off-by: Bernhard Stadlbauer <[email protected]>

* Add tests to `dask` task

Signed-off-by: Bernhard Stadlbauer <[email protected]>

* Remove namespace

Signed-off-by: Bernhard Stadlbauer <[email protected]>

* Update setup.py

Signed-off-by: Bernhard Stadlbauer <[email protected]>

* Add dask to `plugin/README.md`

Signed-off-by: Bernhard Stadlbauer <[email protected]>

* Add README.md for `dask`

Signed-off-by: Bernhard Stadlbauer <[email protected]>

* Top level export of `JopPodSpec` and `DaskCluster`

Signed-off-by: Bernhard Stadlbauer <[email protected]>

* Update docs for images

Signed-off-by: Bernhard Stadlbauer <[email protected]>

* Update README.md

Signed-off-by: Bernhard Stadlbauer <[email protected]>

* Update models after `flyteidl` change

Signed-off-by: Bernhard Stadlbauer <[email protected]>

* Update task after `flyteidl` change

Signed-off-by: Bernhard Stadlbauer <[email protected]>

* Raise error when less than 1 worker

Signed-off-by: Bernhard Stadlbauer <[email protected]>

* Update flyteidl to >= 1.3.2

Signed-off-by: Bernhard Stadlbauer <[email protected]>

* Update doc requirements

Signed-off-by: Bernhard Stadlbauer <[email protected]>

* Update doc-requirements.txt

Signed-off-by: Bernhard Stadlbauer <[email protected]>

* Re-lock dependencies on linux

Signed-off-by: Bernhard Stadlbauer <[email protected]>

* Update dask API docs

Signed-off-by: Bernhard Stadlbauer <[email protected]>

* Fix documentation links

Signed-off-by: Bernhard Stadlbauer <[email protected]>

* Default optional model constructor arguments to `None`

Signed-off-by: Bernhard Stadlbauer <[email protected]>

* Refactor `convert_resources_to_resource_model` to `core.resources`

Signed-off-by: Bernhard Stadlbauer <[email protected]>

* Use `convert_resources_to_resource_model` in `core.node`

Signed-off-by: Bernhard Stadlbauer <[email protected]>

* Incorporate review feedback

Signed-off-by: Eduardo Apolinario <[email protected]>

* Lint

Signed-off-by: Eduardo Apolinario <[email protected]>

Signed-off-by: Bernhard Stadlbauer <[email protected]>
Signed-off-by: Bernhard Stadlbauer <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Co-authored-by: Eduardo Apolinario <[email protected]>
Co-authored-by: Eduardo Apolinario <[email protected]>

* Do not bring dask changes

Signed-off-by: eduardo apolinario <[email protected]>

* Remove readthedocs

Signed-off-by: eduardo apolinario <[email protected]>

* Linting

Signed-off-by: eduardo apolinario <[email protected]>

* Force scipy<1.11.0 is on windows

Signed-off-by: eduardo apolinario <[email protected]>

---------

Signed-off-by: Yee Hing Tong <[email protected]>
Signed-off-by: Yubo Wang <[email protected]>
Signed-off-by: eduardo apolinario <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Fabio Grätz <[email protected]>
Signed-off-by: Yicheng-Lu-llll <[email protected]>
Signed-off-by: byhsu <[email protected]>
Signed-off-by: tnam <[email protected]>
Signed-off-by: ChungYujoyce <[email protected]>
Signed-off-by: Bernhard Stadlbauer <[email protected]>
Signed-off-by: Bernhard Stadlbauer <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Co-authored-by: Yee Hing Tong <[email protected]>
Co-authored-by: Yubo Wang <[email protected]>
Co-authored-by: Yubo Wang <[email protected]>
Co-authored-by: eduardo apolinario <[email protected]>
Co-authored-by: Kevin Su <[email protected]>
Co-authored-by: Fabio M. Graetz, Ph.D <[email protected]>
Co-authored-by: Fabio Grätz <[email protected]>
Co-authored-by: Yicheng-Lu-llll <[email protected]>
Co-authored-by: ByronHsu <[email protected]>
Co-authored-by: byhsu <[email protected]>
Co-authored-by: TomNam <[email protected]>
Co-authored-by: tnam <[email protected]>
Co-authored-by: ChungYujoyce <[email protected]>
Co-authored-by: bstadlbauer <[email protected]>
eapolinario pushed a commit that referenced this pull request Jul 10, 2023
* Add http_proxy to client & Fix deviceflow

RB=3890720

Signed-off-by: byhsu <[email protected]>

* nit

Signed-off-by: byhsu <[email protected]>

* lint!

Signed-off-by: byhsu <[email protected]>

---------

Signed-off-by: byhsu <[email protected]>
Co-authored-by: byhsu <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
eapolinario added a commit that referenced this pull request Jul 12, 2023
* Multi arch imageSpec (#1630)

Multi arch imageSpec (#1630)

Signed-off-by: Eduardo Apolinario <[email protected]>

* Add executor_path and applications_path to spark config (#1634)

* Add executor_path and applications_path to spark config

Signed-off-by: Kevin Su <[email protected]>

* nit

Signed-off-by: Kevin Su <[email protected]>

---------

Signed-off-by: Kevin Su <[email protected]>

* Add support for env vars to pyflyte run (#1617)

* Add support for env vars to pyflyte run

Signed-off-by: Kevin Su <[email protected]>

* bump idl

Signed-off-by: Kevin Su <[email protected]>

* update doc

Signed-off-by: Kevin Su <[email protected]>

* nit

Signed-off-by: Kevin Su <[email protected]>

* nit

Signed-off-by: Kevin Su <[email protected]>

* nit

Signed-off-by: Kevin Su <[email protected]>

---------

Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>

* Fetch task executions in dynamic  (#1636)

* fetch task executions in dynamic

Signed-off-by: Kevin Su <[email protected]>

* nit

Signed-off-by: Kevin Su <[email protected]>

* lint

Signed-off-by: Kevin Su <[email protected]>

---------

Signed-off-by: Kevin Su <[email protected]>

* Added metrics command to pyflyte (#1513)

Signed-off-by: Daniel Rammer <[email protected]>

* Add http_proxy to client & Fix deviceflow (#1611)

* Add http_proxy to client & Fix deviceflow

RB=3890720

Signed-off-by: byhsu <[email protected]>

* nit

Signed-off-by: byhsu <[email protected]>

* lint!

Signed-off-by: byhsu <[email protected]>

---------

Signed-off-by: byhsu <[email protected]>
Co-authored-by: byhsu <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>

* Improve variable names (#1642)

Signed-off-by: byhsu <[email protected]>
Co-authored-by: byhsu <[email protected]>

* Address resolution (#1567)

Signed-off-by: Yee Hing Tong <[email protected]>

* pyflyte run supports pickle (#1646)

Signed-off-by: Kevin Su <[email protected]>

* Wait for the pod plugin instead of flytekit (#1647)

Signed-off-by: eduardo apolinario <[email protected]>
Co-authored-by: eduardo apolinario <[email protected]>

* Beautify deviceflow prompt (#1625)

* Beautify deviceflow prompt

Signed-off-by: byhsu <[email protected]>

* lint!

Signed-off-by: byhsu <[email protected]>

* lint

Signed-off-by: byhsu <[email protected]>

---------

Signed-off-by: byhsu <[email protected]>
Co-authored-by: byhsu <[email protected]>

* Improve flytekit register (#1643)

* Fix pyflyte register

Signed-off-by: byhsu <[email protected]>

* revert

Signed-off-by: byhsu <[email protected]>

* lint

Signed-off-by: byhsu <[email protected]>

---------

Signed-off-by: byhsu <[email protected]>
Co-authored-by: byhsu <[email protected]>

* Pass verify flag to all authenticators (#1641)

Signed-off-by: byhsu <[email protected]>

* Allow annotated FlyteFile as task input argument (#1632)

* fix: Allow annotated FlyteFile as task input argument

Using an annotated FlyteFile type as an input to a task was previously impossible due
to an exception being raised in `FlyteFilePathTransformer.to_python_value`.

This commit applies the fix previously used in `FlyteFilePathTransformer.to_literal`
to permit using annotated FlyteFiles as either inputs and outputs of a task.

Issue: #3424
Signed-off-by: Adrian Rumpold <[email protected]>

* refactor: Unified handling of annotated types in type engine

Issue: #3424
Signed-off-by: Adrian Rumpold <[email protected]>

* fix: Use py3.8-compatible types in type engine tests

Issue: #3424
Signed-off-by: Adrian Rumpold <[email protected]>

---------

Signed-off-by: Adrian Rumpold <[email protected]>

* Use logger instead of print statement in sqlalchemy plugin (#1651)

* use logging info instead of print

Signed-off-by: wirthual <[email protected]>

* isorted files

Signed-off-by: wirthual <[email protected]>

* import root logger from flytekit

Signed-off-by: wirthual <[email protected]>

---------

Signed-off-by: wirthual <[email protected]>

* Map over notebook task (#1650)

* map over notebook

Signed-off-by: Kevin Su <[email protected]>

* nit

Signed-off-by: Kevin Su <[email protected]>

* nit

Signed-off-by: Kevin Su <[email protected]>

* nit

Signed-off-by: Kevin Su <[email protected]>

* tests

Signed-off-by: Kevin Su <[email protected]>

* nit

Signed-off-by: Kevin Su <[email protected]>

* add a flag

Signed-off-by: Kevin Su <[email protected]>

* nit

Signed-off-by: Kevin Su <[email protected]>

* fix tests

Signed-off-by: Kevin Su <[email protected]>

* nit

Signed-off-by: Kevin Su <[email protected]>

* lint

Signed-off-by: Kevin Su <[email protected]>

* Fix tests

Signed-off-by: Kevin Su <[email protected]>

* lint

Signed-off-by: Kevin Su <[email protected]>

---------

Signed-off-by: Kevin Su <[email protected]>

* Support single literals in tiny url (#1654)

Signed-off-by: Yee Hing Tong <[email protected]>

* Add support overriding image (#1652)

Signed-off-by: Kevin Su <[email protected]>

* Fix ability to pass None to task with Optional kwarg, add test (#1657)

Signed-off-by: Fabio Grätz <[email protected]>
Co-authored-by: Fabio Grätz <[email protected]>

* Regenerate plugins requirements

Signed-off-by: eduardo apolinario <[email protected]>

* Regenerate plugins requirements and linting

Signed-off-by: eduardo apolinario <[email protected]>

* Regenerate whylogs requirements

Signed-off-by: eduardo apolinario <[email protected]>

---------

Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Daniel Rammer <[email protected]>
Signed-off-by: byhsu <[email protected]>
Signed-off-by: Yee Hing Tong <[email protected]>
Signed-off-by: eduardo apolinario <[email protected]>
Signed-off-by: Adrian Rumpold <[email protected]>
Signed-off-by: wirthual <[email protected]>
Signed-off-by: Fabio Grätz <[email protected]>
Co-authored-by: Kevin Su <[email protected]>
Co-authored-by: Dan Rammer <[email protected]>
Co-authored-by: ByronHsu <[email protected]>
Co-authored-by: byhsu <[email protected]>
Co-authored-by: Yee Hing Tong <[email protected]>
Co-authored-by: eduardo apolinario <[email protected]>
Co-authored-by: Adrian Rumpold <[email protected]>
Co-authored-by: wirthual <[email protected]>
Co-authored-by: Fabio M. Graetz, Ph.D <[email protected]>
Co-authored-by: Fabio Grätz <[email protected]>
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