-
Notifications
You must be signed in to change notification settings - Fork 7
修復cahce login 的問題#74 #75
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some issue need to be fixed and clarify
SECRET_KEY = red.get("SECRET_KEY") if red.exists( | ||
"SECRET_KEY") else str(os.urandom(32)) | ||
red_auth = redis.StrictRedis.from_url( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't get it, what is the different between red
and red_auth
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't use use red
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
這次為了減少更動,當初存cookie存入redis沒有設定編碼(存bytes),導致把資料拿出來又要經過一波轉碼的過程,個人認為轉碼應該在redis上就做好,不應該撈出來再轉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If so, then it should be a separate PR, and it should not deal in this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
我認為還是在同一個問題中,我並沒有要處理其他舊有的code存入redis是bytes的問題,我而外開了一個是已經會編碼的redis連接只處理cache login 沒檢查cache的事情
當然如果還是覺得有問題
也是能繼續用red
去連線,也只是讀入時要再額外編碼就是
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But @takidog think this problem login cache is server delay main reason in this weeks.
At same time v3 api is developing, and request will down in next week.
I prefer change in v3 api.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@takidog this problem need open issue to tracking
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add comment on that line to express your idea, otherwise others will get the same confused I got
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
已補
src/kuas_api/kuas/cache.py
Outdated
|
||
|
||
def login(username, password): | ||
session = requests.Session() | ||
is_login = {} | ||
|
||
if red_auth.exists(username): | ||
user_redis_cookie = red_auth.get(username) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove duplicate blank line
src/kuas_api/kuas/cache.py
Outdated
if red_auth.exists(username): | ||
user_redis_cookie = red_auth.get(username) | ||
|
||
login_json = json.loads(user_redis_cookie) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return json.loads(user_redis_cookies)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
了解
src/kuas_api/kuas/cache.py
Outdated
|
||
|
||
def login(username, password): | ||
session = requests.Session() | ||
is_login = {} | ||
|
||
if red_auth.exists(username): | ||
user_redis_cookie = red_auth.get(username) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cookie -> cookies
Why use"red_auth" not "red" ? Because "red" return data is bytes , I think get data from redis should be able to use without any decode or encode action.
如題
Fixes #74
cache login 會檢查redis 是否有cookie
cookie 失效時間設定為600秒
wrk test
條件一樣下,有修復與沒修復的差異
關於commit的red or red_auth
從redis library上應該拿出bytes 在做編碼的動作,還是拿出資料就已經是編碼好的
我認為沒有太大差別 (後者較優 1000次下 平均差距:0.5~1ms 可以當作沒有效能差距)
這只是一個hotfix 我不認為在這次修正上把以往所有redis的這個問題修正是個好方法,目前要開發
v3,但近期查詢成績過多,v2勢必要先修復了,這PR只是為了解決cache login 的問題,以不引發其他bug為主。
每個請求都會call cache.login 這問題
要修太複雜
所以 只在cache.login 好好檢查redis上的cookies
要修復整個邏輯要變動的東西太多,用最少的code修復最主要的問題