eak24 / onshapepy

The OnShape Python package.
MIT License
5 stars 1 forks source link

credentials via python does not work #20

Open pmdhazy opened 6 years ago

pmdhazy commented 6 years ago

The instructions at:

https://aguaclara.github.io/onshapepy/configuration.html#configuration

for configuring via python do not work since at the bottom of client.py there is a line which creates a new Client() which calls init which then creates a new Onshape() which throws an error:

self._access_key = creds['access_key'].encode('utf-8') TypeError: 'NoneType' object is not subscriptable

I do not think the instantiate of a singleton object at the end of client.py which happens at import time is a good idea. Code should only be run when user explicitly calls something

eak24 commented 6 years ago

Hi @pmdhazy you are absolutely correct! standard wisdom is to not execute code during import - however, there is one exception, which is when using a module itself as a form of a singleton. When that happens, a given import is gauranteed to only execute code in the top level once. That was my original approach. I could easily be convinced to change it. The way to specify new creds is by setting the creds in the client. for exampl:

from onshapepy.core.client import c
c.update({"creds":<your creds>}) 

What will happen in 0.0.19 is I'll move the whole client "class" into the top level module. All this is to avoid awkwardly explicit passing around of the client because it will now be globally available. That way I can effective remove any need to think about the client unless you want to. In that version, programmatically altering the creds will be as simple as:

import onshapepy.core.client as client
client.creds = {'access_key':.'secret_key':}

@pmdhazy if you have an alternative suggestion that will still allow me to implicitly share client state across multiple modules, please let me know!!

pmdhazy commented 6 years ago

Hi @ethan92429 ,

I am a bit out of practice so cannot propose any alternative at the moment other than vague hand waving about some default factory method.

The main issue I ran into is the code in the library does:

def some_method(a, b, client=Client()):
       ...

which causes it to run at import time (and fall over with the error I reported). If it did::

def some_method(a, b, client=None):
      if client is None:
           client=Client()

then it would have delayed called Client() until actually needed rather than at import time.