Skip to content

Conversation

@congminh1254
Copy link
Member

No description provided.

@congminh1254 congminh1254 requested a review from a team January 19, 2026 20:48
@lukaszsocha2 lukaszsocha2 changed the title feat: shared network client and token storage feat(boxsdk): shared network client and token storage Jan 20, 2026
Note: This will call store_tokens with None values if the callback supports it.
"""
if self._store_tokens is not None:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not just = None ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in python checking a is None is preferred over a ==None

response_object=box_response.json(),
)

def get_authentication(self, *, token_storage=None):
Copy link
Contributor

@lukaszsocha2 lukaszsocha2 Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing typehint for token_storage and returned type


# Developer Token Authentication
if isinstance(oauth, DeveloperTokenAuth):
token = oauth.access_token
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe extract if body to separate private methods for better readability

to a generated SDK Authentication object.
This method supports the following legacy authentication types:
- DeveloperTokenAuth -> BoxDeveloperTokenAuth
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LegacyOAuth2?

oauth._store_tokens(access_token, refresh_token)

token_storage = LegacyTokenStorageAdapter(
get_tokens=get_tokens, store_tokens=store_tokens
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use lambda?

# Pre-populate with existing tokens if available
auth = BoxOAuth(config=config)
access_token, refresh_token = oauth._get_tokens()
if access_token:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed? Wouldn't old token storage be called to get token?

upload_url = getattr(api_config, 'UPLOAD_URL', 'https://upload.box.com/api/2.0')
# Remove version suffix if present
if upload_url.endswith('/2.0'):
upload_url = upload_url[:-4]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extract to a method


return network_session

def get_sdk_gen_client(self, *, auth_options=None, network_options=None):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing typehints

Copy link
Contributor

@lukaszsocha2 lukaszsocha2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move the method implementation to separate file. client.py is already huge. And split implementation into smaller methods. Avoid inner funtions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants