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

Beautify deviceflow prompt #1625

Merged
merged 3 commits into from
May 19, 2023
Merged

Conversation

ByronHsu
Copy link
Collaborator

@ByronHsu ByronHsu commented May 9, 2023

Type

  • Bug Fix
  • Feature
  • Plugin

Complete description

To make deviceflow prompt more prominent

Note: for why verification_url_completed is removed, check #1611

image

@codecov
Copy link

codecov bot commented May 9, 2023

Codecov Report

Merging #1625 (73dd291) into master (c65397d) will decrease coverage by 0.03%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master    #1625      +/-   ##
==========================================
- Coverage   71.04%   71.01%   -0.03%     
==========================================
  Files         335      336       +1     
  Lines       30661    30726      +65     
  Branches     5558     5567       +9     
==========================================
+ Hits        21783    21821      +38     
- Misses       8336     8361      +25     
- Partials      542      544       +2     
Impacted Files Coverage Δ
flytekit/clients/auth/authenticator.py 52.00% <66.66%> (-0.04%) ⬇️

... and 10 files with indirect coverage changes

Copy link
Contributor

@kumare3 kumare3 left a comment

Choose a reason for hiding this comment

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

I think we should start using rich console everywhere. It adjusts automatically to env like jupyter

OR copy paste the following URL: {resp.verification_uri_complete}
"""
)
text = f"To Authenticate, navigate in a browser to the following URL: {click.style(resp.verification_uri, fg='blue', underline=True)} and enter code: {click.style(resp.user_code, fg='blue')}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually let's use rich console

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do you mean using the logger? Looks kinda weird
image

Copy link
Contributor

Choose a reason for hiding this comment

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

ya not the logger, i meant rich.console. but let it be what it was, can you revert and i can approve!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i just reverted

@ByronHsu ByronHsu force-pushed the beautify-deviceflow branch from 7663a38 to 095f2f5 Compare May 11, 2023 04:43
@kumare3
Copy link
Contributor

kumare3 commented May 11, 2023

Some build fails?

pingsutw
pingsutw previously approved these changes May 12, 2023
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.

LGTM, could you add a screenshot in the description?

ByronHsu added 2 commits May 15, 2023 22:10
Signed-off-by: byhsu <[email protected]>
Signed-off-by: byhsu <[email protected]>
@pingsutw pingsutw merged commit a40ccad into flyteorg:master May 19, 2023
@ByronHsu ByronHsu deleted the beautify-deviceflow branch May 29, 2023 01:10
eapolinario pushed a commit that referenced this pull request Jul 10, 2023
* 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]>
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