Skip to content

session-per-request to session-over-time - #16

Closed
runawaycoast wants to merge 8 commits into
mainfrom
DLPLAT-352-connection-module-change
Closed

session-per-request to session-over-time#16
runawaycoast wants to merge 8 commits into
mainfrom
DLPLAT-352-connection-module-change

Conversation

@runawaycoast

@runawaycoastrunawaycoast commented Apr 5, 2022

Copy link
Copy Markdown
Contributor

this PR changed from session-per-request to session-over-time.

previous implementation session-per-request:
Every time we call API with get*, it will create a session within get_session() function.

Current implementation session-over-time:

  1. System creates a global variable session, initially be assigned with None.
  2. Once get_session() has been called for the first time, it will create the global session and setup retry strategy for it. (*)
  • Retry strategy can only be setup on session level. If we create session in global, the code will not respect the APIConfig retry strategy (ApiConfig could be designed to be a config object to pass into each request, instead of make functions depend on a global one)

con:

  • clients cannot change the retry strategy after the first request, (System can add a listener on ApiConfig, and every time clients change retry config, system generate a new session)

Screen Shot 2022-04-05 at 10 01 06 PM

15501005001000
non-reuse - function count3571083986328657771384827553722710727727
reuse - function count3571193968168433511339501530870110270201
15501005001000
non-reuse - second0.550.691.422.438.7116.768
reuse - second0.530.610.91.384.678.68

@runawaycoast
runawaycoastforce-pushed the DLPLAT-352-connection-module-change branch from 58e224f to d910538CompareApril 5, 2022 20:35
@runawaycoastrunawaycoast changed the title Dlplat 352 connection module changesession-per-request to session-over-timeApr 6, 2022
@runawaycoast

Copy link
Copy Markdown
ContributorAuthor

Close this pr in favour of #22

Sign up for freeto 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.

1 participant

@runawaycoast