Dev/sso login ux #608

vaimdev · 2024-10-28T10:34:33Z

An attempt to improve UI/UX to the databricks dashboard with SSO usage
Added a few helper functions, so that the databricks dashboard can keep a simpler flow and UX.
Example databricks notebooks that utilize the new helper functions
https://dbc-ac10efa1-77c5.cloud.databricks.com/editor/notebooks/3715908489050384?o=7408253037638600#command/1247495116400904

3 pre-steps for the SSO login using the helper functions (with different cells), followed by the actual plotting python code

graphistry.databricks_sso_login_init()

graphistry.databricks_sso_login(server="test-2-40-74-dbt.grph.xyz", org_name="louiedev")

graphistry.databricks_sso_wait_for_token()

lmeyerov · 2024-10-30T16:27:44Z

Can you share the code sample in the PR?

(Trouble accessing link, and discussion should be here)

lmeyerov · 2024-10-30T16:52:54Z

@vaimdev @DataBoyTX This looks strange from a dev UX layer, afaict:

Demo

If I understand correctly, the below is the intended setup:

cell 1:

clear_output()
graphistry.databricks_sso_login_init()

cell 2:

clear_output()
graphistry.databricks_sso_login(server="test-2-40-74-dbt.grph.xyz", org_name="louiedev")

cell 3:

clear_output()
graphistry.databricks_sso_wait_for_token()

cell 4:

g.plot()

Discussion

I understand wanting new underlying primitives, but how to expose it to users should be minimal, clean, integrated, smart defaults, etc

Why is this 4 cells instead of 2?
why clear_output() ? Can we make that default-on parameter for the auth step with pass-in overrides?
I believe part of this weird path is SSO differences in how databricks headless, databricks notebooks, and databricks dashboards work; are these primitives for dashboards only, or notebook + headless too?

Examples of a two-cell: graphistry.register(sso='databricks').... g.plot():

lmeyerov · 2024-10-30T16:53:46Z

This PR should also include sample ipynb + docs updates. Sounds like we need to iterate first on interface definitions tho.

The current is https://pygraphistry.readthedocs.io/en/latest/demos/demos_databases_apis/databricks_pyspark/graphistry-notebook-dashboard.html

Maybe dashboards or auth should split out?

lmeyerov · 2024-10-30T16:57:05Z

graphistry/pygraphistry.py

+    class DatabricksHelper():
+
+        @staticmethod
+        def sso_repeat_get_token(repeat, wait):


public methods should have .rst-friendly docstrs

also, we're trying to make all new code mypy-typed

ok, noted. I will add them in.

vaimdev · 2024-10-31T10:00:23Z

@vaimdev @DataBoyTX This looks strange from a dev UX layer, afaict:

Demo

If I understand correctly, the below is the intended setup:

cell 1:
clear_output()
graphistry.databricks_sso_login_init()
cell 2:
clear_output()
graphistry.databricks_sso_login(server="test-2-40-74-dbt.grph.xyz", org_name="louiedev")
cell 3:
clear_output()
graphistry.databricks_sso_wait_for_token()
cell 4:
g.plot()
Discussion

I understand wanting new underlying primitives, but how to expose it to users should be minimal, clean, integrated, smart defaults, etc

Why is this 4 cells instead of 2?

why clear_output() ? Can we make that default-on parameter for the auth step with pass-in overrides?

I believe part of this weird path is SSO differences in how databricks headless, databricks notebooks, and databricks dashboards work; are these primitives for dashboards only, or notebook + headless too?

Examples of a two-cell: graphistry.register(sso='databricks').... g.plot():

4 cells instead of 2, mainly because each cell will only display the output at the end of running of 1 cell. Previously, there are a few logic written into 1 cell and the output only be displayed after the cell is run which make the messages looks strange,
in some cases, the refresh token will take place and the message shown is not correct.
breaking it into 4 cells will make the the UX consistent and able to control the logic in the case of refresh token, to display the correct message

why clear_output() ? Can we make that default-on parameter for the auth step with pass-in overrides?

clear_output() is not necessary, can be removed.

I believe part of this weird path is SSO differences in how databricks headless, databricks notebooks, and databricks dashboards work; are these primitives for dashboards only, or notebook + headless too?

Mainly for databricks dashboard but tested in both databricks notebook and databricks dashboard.

lmeyerov · 2024-11-02T00:18:22Z

graphistry/pygraphistry.py

+                        try:
+                            PyGraphistry.refresh()
+                        except Exception:
+                            pass


this error case handling may need thinking?

lmeyerov · 2024-11-02T00:21:01Z

graphistry/pygraphistry.py

+        def databricks_sso_login_init(wait=20):
+            from IPython.display import display, HTML, clear_output
+
+            def init_login(wait):


vaimdev · 2024-11-02T00:29:06Z

suggestion from Leo:

Show a message on Graphistry server, when SSO login from pygraphistry

…ich originally just a refresh()

… will be displayed.

…lid token

lmeyerov · 2024-11-08T22:02:03Z

@vaimdev FYI ci lint issues

vaimdev · 2024-11-09T01:42:48Z

@vaimdev FYI ci lint issues

Yes, noted, added some debugging code, now it is done and cleaning up code.

vaimdev · 2024-11-09T15:48:14Z

Loom videos to demo the changes

Databricks Notebook
Not login - https://www.loom.com/share/b48939faf5ba4578bc66218cb89cacb9?sid=c72bae54-9ce9-4128-8f2a-ebd128e70dad

Log in - https://www.loom.com/share/2313bb86ef454a9f9d55c1535c785457?sid=42f46537-f726-4a89-a98d-96f762e0cf51

Dashboard
https://www.loom.com/share/68add6bd0c2348e0b0fc0b19abbf4939?sid=2a878545-e92c-4dca-843f-2f297d4fd52f

lmeyerov · 2024-11-09T16:10:07Z

graphistry/pygraphistry.py

+    @staticmethod
+    def sso_repeat_get_token(repeat: int = 20, wait: int = 5):
+        """ Function to repeatly call to obtain the jwt token after sso login
+        Function to obtain the JWT token after SSO login


@sabizmil can you check all public method docstr as these show up in the readthedocs?

Spelling, grammar, docstr types, and examples, following format of the rest

Updated in the latest commit

sabizmil

Left comments with updated message content.

sabizmil · 2024-11-11T19:35:39Z

graphistry/pygraphistry.py

@@ -271,7 +277,7 @@ def _handle_auth_url(auth_url, sso_timeout, sso_opt_into_type):
        if in_ipython() or in_databricks() or sso_opt_into_type == 'display':  # If run in notebook, just display the HTML
            # from IPython.core.display import HTML
            from IPython.display import display, HTML
-            display(HTML(f'<a href="{auth_url}" target="_blank">Login SSO</a>'))
+            display(HTML(f'<a href="{auth_url}" target="_blank">Login SSO</a><br /><span>Please click the above URL to open browser to login</span>'))


<a href="{auth_url}" target="_blank" style="display: inline-block; margin-top: 10px; padding: 10px 20px; font-size: 16px; font-weight: bold; color: #fff; background-color: #007bff; text-decoration: none; border-radius: 5px;">Login with SSO</a> Please click the button above to open the browser and log in.

sabizmil · 2024-11-11T19:38:25Z

graphistry/pygraphistry.py

+        if not PyGraphistry.api_token():
+            msg_text = '....'
+            if not PyGraphistry.sso_repeat_get_token(repeat, wait):
+                msg_text = f'{msg_text}\nFailed to get token after {repeat*wait} seconds ....'


msg_text = f'{msg_text}\nUnable to retrieve token after {repeat*wait} seconds. Please try logging in again.'

sabizmil · 2024-11-11T19:38:39Z

graphistry/pygraphistry.py

+            if not PyGraphistry.sso_repeat_get_token(repeat, wait):
+                msg_text = f'{msg_text}\nFailed to get token after {repeat*wait} seconds ....'
+                if not fail_silent:
+                    msg = f"Failed to get token after {repeat*wait} seconds. Please re-run the login process"


msg = f"Token retrieval failed after {repeat*wait} seconds. Please try logging in again."

sabizmil · 2024-11-11T19:39:16Z

graphistry/pygraphistry.py

+                        display_message_html(f"<strong>{msg}</strong>")
+                    raise Exception(msg) 
+                else:
+                    msg_text = f'{msg_text}\nGot token'


msg_text = f'{msg_text}\nToken retrieved successfully.'

sabizmil · 2024-11-11T19:39:20Z

graphistry/pygraphistry.py

+                    print(msg_text)
+                    return
+
+            msg_text = f'{msg_text}\nGot token'


msg_text = f'{msg_text}\nToken retrieved successfully.'

sabizmil · 2024-11-11T19:40:49Z

graphistry/pygraphistry.py

+            if not PyGraphistry.sso_repeat_get_token(repeat, wait):
+                msg_html = f'{msg_html}<br /><strong>Failed to get token after {repeat*wait} seconds .... </strong>'
+                if not fail_silent:
+                    raise Exception(f"Failed to get token after {repeat*wait} seconds. Please re-run the login process") 


raise Exception(f"Unable to retrieve token after {repeat*wait} seconds. Please try logging in again.")

sabizmil · 2024-11-11T19:40:59Z

graphistry/pygraphistry.py

+            msg_html = f'{msg_html}<br /><strong>Got token</strong>'
+            display(HTML(msg_html))
+        else:
+            display(HTML('<br /><strong>Token is valid, no waiting required.</strong>'))


display(HTML(' Token is valid; no further action needed.'))

sabizmil · 2024-11-11T19:41:10Z

graphistry/pygraphistry.py

+            if not is_valid:
+                print("***********token not valid, refresh token*****************")
+                if display_mode == 'html':
+                    display(HTML('<br /><strong>Refresh token ....</strong>'))


display(HTML(' Refreshing token...'))

sabizmil · 2024-11-11T19:41:20Z

graphistry/pygraphistry.py

+            else:
+                print("Token is still valid")
+                if display_mode == 'html':
+                    display(HTML('<br /><strong>Token is still valid ....</strong>'))


display(HTML(' Token is still valid; no re-login required.'))

sabizmil · 2024-11-11T19:41:31Z

graphistry/pygraphistry.py

+
+        if required_login:
+            print("***********Prepare to sign in*****************")            
+            msg_html = f'<br /><strong>Prepare to sign in ....</strong><br><strong>Please Login with the link appear later. Waiting for success login for {repeat*wait} seconds, please login within {wait} seconds....</strong><br /><strong>Please close the browser tab and come back to dashboard....</strong>'


msg_html = f' Preparing to sign in... Use the link provided below to log in. Please complete the login within {wait} seconds. After logging in, you may close the browser tab and return to the dashboard.'

vaimdev added 7 commits October 17, 2024 13:32

fix (typecheck): fix typecheck issue

89f3d5b

Merge branch 'master' into dev/sso_login_ux

cdeaf63

feat: Add databricks helper class and functions

dbad147

fix (refresh token, lint): Fix refresh token and lint issue

799e93b

fix (lint): fix lint issue

e1737d8

fix (databricks helper): fix missing function definition

caa7809

fix (databricks helpers): fix function call namespace

71c5eb1

vaimdev requested a review from DataBoyTX October 30, 2024 09:37

vaimdev marked this pull request as ready for review October 30, 2024 09:37

lmeyerov reviewed Oct 30, 2024

View reviewed changes

lmeyerov reviewed Nov 2, 2024

View reviewed changes

vaimdev added 13 commits November 6, 2024 00:02

wip (sso logic): Add SSO flow token verification in plot function, wh…

c187ead

…ich originally just a refresh()

wip (sso): fix whether to throw exception when fail to get token

412e944

wip (sso): fix message display logic

9283e8a

fix (sso): fix flow

be26dee

fix (sso): fix display message flow

5ea4841

fix (sso): Graph is not shown in dashboard due to only the first HTML…

8a482b5

… will be displayed.

wip (sso): fix syntax error

ab2794c

wip (sso): debugging code

5b35284

wip (debug): add debug print

8355b0f

wip (fix): syntax error

1a2ab33

wip (debug):debugging print

a536648

wip (debug): debugging print

dd29430

wip (sso): attempt to display error in iframe

36fcfea

vaimdev added 3 commits November 9, 2024 00:20

wip (databricks): return error msg html in the case of plot with inva…

4820620

…lid token

wip (sso): display html message before throw exception

2d6ad0b

wip(sso): fix error

05d0dff

vaimdev added 2 commits November 9, 2024 20:05

wip (sso): fix syntax error

fd46826

wip (sso): clean up code

4816fd6

lmeyerov reviewed Nov 9, 2024

View reviewed changes

sabizmil requested changes Nov 11, 2024

View reviewed changes

sabizmil and others added 2 commits November 11, 2024 12:32

Update docstr for new/updated methods

bff2459

fix (lint): fix lint issue

4b0b129

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dev/sso login ux #608

Dev/sso login ux #608

vaimdev commented Oct 28, 2024 •

edited

Loading

lmeyerov commented Oct 30, 2024

lmeyerov commented Oct 30, 2024

lmeyerov commented Oct 30, 2024 •

edited

Loading

lmeyerov Oct 30, 2024

lmeyerov Oct 30, 2024

vaimdev Oct 31, 2024

vaimdev commented Oct 31, 2024 •

edited

Loading

Demo

Discussion

lmeyerov Nov 2, 2024

lmeyerov Nov 2, 2024

vaimdev commented Nov 2, 2024

lmeyerov commented Nov 8, 2024

vaimdev commented Nov 9, 2024

vaimdev commented Nov 9, 2024

lmeyerov Nov 9, 2024

sabizmil Nov 11, 2024

sabizmil left a comment

sabizmil Nov 11, 2024

sabizmil Nov 11, 2024

sabizmil Nov 11, 2024

sabizmil Nov 11, 2024

sabizmil Nov 11, 2024

sabizmil Nov 11, 2024

sabizmil Nov 11, 2024

sabizmil Nov 11, 2024

sabizmil Nov 11, 2024

sabizmil Nov 11, 2024

Dev/sso login ux #608

Are you sure you want to change the base?

Dev/sso login ux #608

Conversation

vaimdev commented Oct 28, 2024 • edited Loading

lmeyerov commented Oct 30, 2024

lmeyerov commented Oct 30, 2024

Demo

Discussion

lmeyerov commented Oct 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vaimdev commented Oct 31, 2024 • edited Loading

Demo

Discussion

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vaimdev commented Nov 2, 2024

lmeyerov commented Nov 8, 2024

vaimdev commented Nov 9, 2024

vaimdev commented Nov 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sabizmil left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vaimdev commented Oct 28, 2024 •

edited

Loading

lmeyerov commented Oct 30, 2024 •

edited

Loading

vaimdev commented Oct 31, 2024 •

edited

Loading