box / box/box-python-sdk

Improve Oauth2 API for handling tokens persistence with SQL

オープン
#207 コメント 2 件 リアクション 0 件 担当者 0 名 GitHub で見る
enhancement
主要言語
Python
スター
459
フォーク
223
平均マージ
8時間 57分
マージ済み PR(30日)
13

説明

I'm trying to accommodate storing my tokens in an SQL database, and trying to extend the `OAuth2` class to enable storing and retrieving the tokens from the database. The `_refresh_lock` is something that is naturally also managed by the same database. (You have a conceptually similar implementation for redis with `RedisManagedOAuth2`)

What you would typically do with SQL in order to lock a specific row to update it (exactly what we want to do here) is use the `SELECT ... FOR UPDATE` command, which locks and retrieves the data at the same time

The existing code where the lock is used (https://github.com/box/box-python-sdk/blob/6597e930929f3ee49d4930a53304277167c0a4db/boxsdk/auth/oauth2.py#L201-L202 and https://github.com/box/box-python-sdk/blob/6597e930929f3ee49d4930a53304277167c0a4db/boxsdk/auth/oauth2.py#L305-L306) looks like this :

```python
def refresh():
with self.refresh_lock:
tokens = self.get_tokens()
# ... proceed with logic to update them
```

What I'm looking for is this

```python
def refresh():
with self.select_for_update(...) as tokens:
# ... proceed with logic
```

Well, I can't really extend the class to achieve that unless I rewrite the whole methods `refresh` and `revoke`. I think a better API to express the need to "lock tokens and retrieve them" is with its own context manager function, that can have a default implementation that can be overridden. This contextmanager can be trivially rewritten to use select for update

```python
def refresh():
with self.lock_tokens(...) as tokens:
# ... proceed with logic

@contextmanager
def lock_tokens():
with self.refresh_lock:
yield self.get_tokens()
```

コントリビューションガイド

コントリビューションガイドを開く

評価

この issue はまだ評価されていません。

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。