flyteorg / flyte

Scalable and flexible workflow orchestration platform that seamlessly unifies data, ML and analytics stacks.
https://flyte.org
Apache License 2.0
5.75k stars 655 forks source link

[BUG] Flyte URL lookup not referencing client if config missing #368

Closed wild-endeavor closed 4 years ago

wild-endeavor commented 4 years ago

Describe the bug This should not happen

(flytekit37) user-mbp151:~ [flyte] $ FLYTE_PLATFORM_AUTH=True ipython
Python 3.7.5 (default, Nov  1 2019, 02:16:32)
Type 'copyright', 'credits' or 'license' for more information
IPython 7.15.0 -- An enhanced Interactive Python. Type '?' for help.

In [1]: from flytekit.configuration.platform import URL

In [2]: URL.get()
---------------------------------------------------------------------------
FlyteAssertion                            Traceback (most recent call last)
<ipython-input-2-7542f11ae7ca> in <module>
----> 1 URL.get()

~/go/src/github.com/lyft/flytekit/flytekit/configuration/common.py in get(self)
    189             val = self._fallback.get()
    190         if self._validator:
--> 191             self._validator(val)
    192         return val
    193

~/go/src/github.com/lyft/flytekit/flytekit/configuration/common.py in _validate_not_null(self, val)
    215                 "No configuration set for [{}] {}.  This is a required configuration.".format(
    216                     self._section,
--> 217                     self._key
    218                 )
    219             )

FlyteAssertion: No configuration set for [platform] url.  This is a required configuration.

In [3]: from flytekit.clients.friendly import SynchronousFlyteClient

In [4]: c = SynchronousFlyteClient('flyte.lyft.net')
---------------------------------------------------------------------------
FlyteAssertion                            Traceback (most recent call last)
<ipython-input-4-790d356548e3> in <module>
----> 1 c = SynchronousFlyteClient('flyte.lyft.net')

~/go/src/github.com/lyft/flytekit/flytekit/clients/raw.py in __init__(self, url, insecure, credentials, options)
    143         self._metadata = None
    144         if _AUTH.get():
--> 145             self.force_auth_flow()
    146
    147     def set_access_token(self, access_token):

~/go/src/github.com/lyft/flytekit/flytekit/clients/raw.py in force_auth_flow(self)
    152     def force_auth_flow(self):
    153         refresh_handler_fn = _get_refresh_handler(_creds_config.AUTH_MODE.get())
--> 154         refresh_handler_fn(self)
    155
    156     ####################################################################################################################

~/go/src/github.com/lyft/flytekit/flytekit/clients/raw.py in _refresh_credentials_standard(flyte_client)
     27     """
     28
---> 29     _credentials_access.get_client().refresh_access_token()
     30     flyte_client.set_access_token(_credentials_access.get_client().credentials.access_token)
     31

~/go/src/github.com/lyft/flytekit/flytekit/clis/auth/credentials.py in get_client()
     48     if _authorization_client is not None and not _authorization_client.expired:
     49         return _authorization_client
---> 50     authorization_endpoints = get_authorization_endpoints()
     51
     52     _authorization_client =\

~/go/src/github.com/lyft/flytekit/flytekit/clis/auth/credentials.py in get_authorization_endpoints()
     58
     59 def get_authorization_endpoints():
---> 60     discovery_endpoint = _get_discovery_endpoint(_HTTP_URL.get(), _URL.get(), _INSECURE.get())
     61     discovery_client = _DiscoveryClient(discovery_url=discovery_endpoint)
     62     return discovery_client.get_authorization_endpoints()

~/go/src/github.com/lyft/flytekit/flytekit/configuration/common.py in get(self)
    189             val = self._fallback.get()
    190         if self._validator:
--> 191             self._validator(val)
    192         return val
    193

~/go/src/github.com/lyft/flytekit/flytekit/configuration/common.py in _validate_not_null(self, val)
    215                 "No configuration set for [{}] {}.  This is a required configuration.".format(
    216                     self._section,
--> 217                     self._key
    218                 )
    219             )

FlyteAssertion: No configuration set for [platform] url.  This is a required configuration.

Expected behavior If the user was kind enough to supply the URL in the creation of the client, we should use it to override any config settings, including environment variables.

Flyte component

To Reproduce Steps to reproduce the behavior: See above

Screenshots NA

Environment Flyte component

Additional context NA

wild-endeavor commented 4 years ago

just FYI @honnix since you've been looking around this code recently.

wild-endeavor commented 4 years ago

Closing the issue for now - put in a patch that should address this.