Skip to content
This repository has been archived by the owner on Sep 12, 2022. It is now read-only.

Fix tacc timeout feature #687

Merged
merged 4 commits into from
Aug 29, 2019
Merged

Conversation

cdosborn
Copy link
Contributor

@cdosborn cdosborn commented Sep 28, 2018

Description

Problem

Several tas api endpoints regularly take more than 5 seconds (ex. getting all projects) and were timing out

Solution

Refactor timeout feature so that the one place that needs a timeout can specify it and handle a timeout exception without affect all other uses of the tas api.

See the commit messages for a longer explanation of the refactor

Checklist before merging Pull Requests

  • New test(s) included to reproduce the bug/verify the feature
  • Add an entry in the changelog
  • Reviewed and approved by at least one other contributor.
  • Change to variables in clank

@cdosborn cdosborn force-pushed the fix-tacc-timeout-feature branch 4 times, most recently from cafa211 to f4048cf Compare October 2, 2018 18:39
@cdosborn cdosborn self-assigned this Oct 2, 2018
@coveralls
Copy link

coveralls commented Oct 2, 2018

Coverage Status

Coverage increased (+0.005%) to 39.289% when pulling e367170 on cdosborn:fix-tacc-timeout-feature into 2ea5f57 on cyverse:master.

@cdosborn cdosborn force-pushed the fix-tacc-timeout-feature branch 2 times, most recently from 2fcfc66 to 3faa553 Compare October 5, 2018 23:18
Several changes here that are all related. The original desired behavior for
the timeout feature was to ensure that api/v1/profile didn't take an
indefinite amount of time in user validation.

We originally accomplished that by putting a timeout of 5 seconds into all
calls to read from the tas api. This was wrong because there are some calls
which rightfully should take more time (like getting all projects).

Th purpose of this change is to add support for timeouts so that the one
place that needed it could use it w/o affecting all the other places.

This refactor allows you construct a driver with a specific timeout. That way
the one place (user validation) could construct a driver with a short timeout,
and handle a timeout exception appropriately.

This change introduces private methods _tacc_api_get and _tacc_api_post to the
driver so that it can provide a timeout in one place, and pass things like
tacc_password once. The tacc_api_get and tacc_api_post methods have been
changed to support timeout. They will eventually be incorporated into the
driver.
@simpsonw
Copy link
Contributor

This is working for me locally and the tests passed after I fixed the merge conflict in the changelog. Merging PR.

@simpsonw simpsonw merged commit f965a00 into cyverse:master Aug 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants