Implement OAuth device code flow #32
Conversation
…apper out for reuse, add lock on get_token
# Conflicts: # pubweb/__init__.py # pubweb/api/auth/iam.py # pubweb/api/auth/oauth_client.py # pubweb/api/auth/username.py # pubweb/api/clients/api.py # pubweb/api/clients/portal.py # pubweb/api/models/dataset.py # pubweb/api/models/file.py # pubweb/api/services/base.py # pubweb/api/services/file.py # pubweb/auth/__init__.py # pubweb/cli/controller.py # pubweb/models/project.py # setup.py
sminot
left a comment
sminot
left a comment
There was a problem hiding this comment.
I wish I had more constructive things to add! From what little I can understand, it looks good to me.
| class Constants: | ||
| home = os.environ.get('PW_HOME', '~/.pubweb') | ||
| config_path = Path(home, 'config.ini').expanduser() | ||
| default_base_url = "pubweb.cloud" |
There was a problem hiding this comment.
Should this be configurable as an environment variable?
| ini_config.read(str(Constants.config_path.absolute())) | ||
| main_config = ini_config['General'] | ||
| auth_method = main_config.get('auth_method') | ||
| base_url = (os.environ.get('PW_BASE_URL') or |
There was a problem hiding this comment.
Oh I see, it is configurable here. I suppose I expected it to follow the same pattern as PW_HOME but now I see it doesn't need to
| self.auth_endpoint = f'https://api.{self.base_url}/auth' | ||
|
|
||
| try: | ||
| info_resp = requests.get(f'{self.rest_endpoint}/info/system') |
There was a problem hiding this comment.
Yeah but it only has like 2 functions
setup.py
Outdated
| "jsonschema>=4.6.1", | ||
| "s3fs==0.4.2", | ||
| "fsspec==2022.10.0" | ||
| "fsspec==2022.10.0", |
There was a problem hiding this comment.
I keep having a hard time installing the package. Do the versions all have to be so explicitly hardcoded?
There was a problem hiding this comment.
Ah ok yeah I'll fix that
pubweb/cli/controller.py
Outdated
| from pubweb.api.config import AuthConfig, save_config, load_config | ||
| from pubweb.file_utils import get_files_in_directory, check_dataset_files | ||
| from pubweb.file_utils import check_dataset_files | ||
| from pubweb.file_utils import get_files_in_directory |
There was a problem hiding this comment.
What was the reason for splitting these 2 imports apart?
There was a problem hiding this comment.
no probably a merge artifact
Uh oh!
There was an error while loading. Please reload this page.