Skip to content

Service timeouts handling for codesets#98

Merged
lsipii merged 7 commits intomainfrom
feat/codesets-service-timeouts-handling
Oct 25, 2023
Merged

Service timeouts handling for codesets#98
lsipii merged 7 commits intomainfrom
feat/codesets-service-timeouts-handling

Conversation

@lsipii
Copy link
Copy Markdown
Contributor

@lsipii lsipii commented Oct 23, 2023

  • extends the PR Add external service requests timeout handling to the security features #97 by adding timeouts handling to the codesets-dependency requests
    • with a timeout value of 9 secs
    • configured at the appsettings.json path Services:Codesets:ServiceRequestTimeoutInMilliseconds
    • for local development environment the max set as 30 secs:
      • the local codesets service does not have initial caching like the live environments should do
  • refactors the codesets -related repositories:
    • use codesets-service (shared http-client, settings)

@lsipii lsipii changed the base branch from main to feat/add-security-service-timeouts-handling October 23, 2023 21:33
@lsipii lsipii marked this pull request as ready for review October 24, 2023 10:15
@lsipii lsipii requested a review from LauriGofore October 24, 2023 10:16
Copy link
Copy Markdown
Contributor

@LauriGofore LauriGofore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Mihin nämä sekuntimäärät perustuu? 9 vs 6 sekuntia, mikä tais olla default tuossa emoPR:ssä

@lsipii
Copy link
Copy Markdown
Contributor Author

lsipii commented Oct 24, 2023

Mihin nämä sekuntimäärät perustuu? 9 vs 6 sekuntia, mikä tais olla default tuossa emoPR:ssä

Lukujen perusteet on että niiden pitää olla pienempiä kuin users-api:n oma max-timeout (aattelin että se voisi olla 15s) ja sen lisäksi ne on ihan vain hattu-arvoita, että mitä kyseisessä käyttötapauksessa saisi maksimissaan (huonoimmassakin tapauksessa) kysely kestää ennen kuin tulkitaan että kyseessä on virhetilanne.

Voisikin laittaa tähän codesetsille tuon luvun konffiin vastaavasti kuin on tuolla toisaalla.

@lsipii lsipii changed the base branch from feat/add-security-service-timeouts-handling to main October 24, 2023 13:27
@lsipii
Copy link
Copy Markdown
Contributor Author

lsipii commented Oct 24, 2023

Lisätty lokaalia devausta varten codesetsille pidennetty timeout-aika koska siellä ei ekoilla kutsuilla käytössä samaa välimuistiratkaisua kuin livessä.

@lsipii lsipii merged commit b0bad35 into main Oct 25, 2023
@lsipii lsipii deleted the feat/codesets-service-timeouts-handling branch October 25, 2023 07:42
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.

2 participants