Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 8 commits into from

Conversation

runawaycoast
Copy link
Contributor

@runawaycoast runawaycoast commented Apr 5, 2022

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

  1 5 50 100 500 1000
non-reuse - function count 357108 398632 865777 1384827 5537227 10727727
reuse - function count 357119 396816 843351 1339501 5308701 10270201
             
  1 5 50 100 500 1000
non-reuse - second 0.55 0.69 1.42 2.43 8.71 16.768
reuse - second 0.53 0.61 0.9 1.38 4.67 8.68

@runawaycoast runawaycoast force-pushed the DLPLAT-352-connection-module-change branch from 58e224f to d910538 Compare April 5, 2022 20:35
@runawaycoast runawaycoast changed the title Dlplat 352 connection module change session-per-request to session-over-time Apr 6, 2022
@runawaycoast
Copy link
Contributor Author

Close this pr in favour of #22

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.

1 participant