alex-oleshkevich / starsessions

Advanced sessions for Starlette and FastAPI frameworks
MIT License
98 stars 11 forks source link

Question about a potentially confusing comment #68

Closed jrobbins-LiveData closed 3 weeks ago

jrobbins-LiveData commented 1 year ago

Line 93 says no matter whether it was initially empty, but lines 88 through 91 seem to have already dealt with the "initially empty" case without removing the cookie or clearing the storage. This made me unsure as to what the comment on line 93 means.

https://github.com/alex-oleshkevich/starsessions/blob/af1c5575223010d27ee600327ae67a745afbfd61/starsessions/middleware.py#L87-L95

alex-oleshkevich commented 1 year ago

initially_empty is True when session data is not loaded from the storage. In this case, we just do nothing.

In the next lines, we work with session data loaded from the storage or modified via request.session. This data can be empty {} which means that we can destroy session and remove the cookie.

jrobbins-LiveData commented 1 year ago

initially_empty seems to be set True at then end of the handler's load method, in the case where the storage was empty. It makes sense to me that in that case we do nothing.

In the next lines, the load method must have initially seen data, setting initially_empty to False. What do the words in the comment on line 93 (no matter whether it was initially empty) mean, since the data was not initially empty here? It seems that the session data could only have been cleared or otherwise emptied by the user.

In other words, it seems that by line 93, the session data was loaded, was not empty, and then became empty. The use of the phrase "initially empty" here seems confusing, and seems to refer to the case already handled at lines 88-91.

alex-oleshkevich commented 3 weeks ago

I removed that comment.