DagsHub / streaming-client

MIT License
2 stars 0 forks source link

Fix data engine client ignoring token env variable #28

Closed guysmoilov closed 1 year ago

guysmoilov commented 1 year ago

There is a lot of copy-pasta of this snippet around the codebase:

dagshub.common.config.token or dagshub.auth.get_token()

This should be refactored. In the meantime, here's a proposed fix specifically for the new data engine code which ignored config obtained from env variables or otherwise.

guysmoilov commented 1 year ago

Refactor strategy IMO:

  1. Choose which of the two packages is lower level, config or auth.
  2. Only call the higher level package from all other places in the codebase to get info like tokens, username, password etc.
  3. Or maybe even provide a higher level abstraction like a HTTP Basic Auth object, or a returned tuple of (user, pass) where either one of the returned tuple values might be None
  4. IMO auth is lower level, you could think about it as a "Getting and storing tokens utility function library" and config as an orchestrator that decides when it should be called vs. using other methods