Adapt oscar_python to the new version #40
Conversation
oscar_python/client.py
Outdated
| try: | ||
| with open(svc["script"]) as s: | ||
| fdl_directory = os.path.dirname(fdl_path) | ||
| script_path = fdl_directory + "/" + svc["script"] |
There was a problem hiding this comment.
Make sure that if the user specifies an absolute path for the shell script in the FDL this is respected. If it is a relative path, then you join fdl_directory with the relative path.
oscar_python/_oidc.py
Outdated
|
|
||
| @staticmethod | ||
| def refresh_access_token(refresh_token, scopes, token_endpoint): | ||
| def refresh_access_token(refresh_token, scopes, token_endpoint, client_id): |
There was a problem hiding this comment.
Please consider whether adding client_id as a new parameter could cause a backward compatibility issue. If we want it to continue working as before, we could add a default value.
oscar_python/client.py
Outdated
| else: | ||
| fdl_directory = os.path.dirname(fdl_path) | ||
| script_path = fdl_directory + "/" + svc["script"] | ||
| print(script_path) |
There was a problem hiding this comment.
Is this print necessary? Was it left over from a debugging process? I suggest removing it or using an appropriate logging method.
oscar_python/client.py
Outdated
| script_path = svc["script"] | ||
| else: | ||
| fdl_directory = os.path.dirname(fdl_path) | ||
| script_path = fdl_directory + "/" + svc["script"] |
There was a problem hiding this comment.
It's better to use os.path.join(fdl_directory, svc['script']). Using simple concatenation with + can cause issues with double slashes //.
| with open(script_path) as s: | ||
| svc["script"] = s.read() | ||
| except IOError: | ||
| raise Exception("Couldn't read script") |
There was a problem hiding this comment.
It's better to preserve the original exception message to improve debugging.
try:
#whatever
except IOError as e:
raise Exception("Couldn't read script") from e
No description provided.