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

limit safe_request retries #32

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jpetermans
Copy link

Prevent unnecessary requests to the reservation server when reservation data is incorrect or the server is unavailable. Multiple repeat requests only necessary for checkin().

@pyro2927, I set the number of retries under checkin() based on CHECKIN_EARLY_SECONDS and CHECKIN_INTERVAL_SECONDS. Seemed like a sane window for retries, but feel free to change. With this and recent your refactor I put some static settings in the init of the reservation class. This made that info easier to reference in both files in this commit.

@jpetermans
Copy link
Author

@pyro2927 I had a chance for a test with this PR and I think I've got the retries on the wrong request to the server. Currently multiple tries are only on the final attempt in checkin. However, it seems the request from get_checkin_data also fails with a BAD_REQUEST if done too early for check-in and the script exits early.

I'm not sure if just get_checkin_data needs to run multiple times if a little early or if the final check in attempt in checkin also does. Can you confirm if one or the other, or both need multiple retries if requests are made slightly earlier than checkin_time?

@pyro2927
Copy link
Owner

pyro2927 commented Mar 3, 2019

@jpetermans I've found it useful for both. Southwest seems to lie more or less, as I wouldn't consider an early-yet-valid to checkin to warrant BAD_REQUEST, yet they seem to. You can keep slinging the same request at them and eventually it'll work. Not sure a better approach on how to deal with it outside of waiting X seconds AFTER the checkin window, but that feels to go against the whole point of the script.

@codeclimate
Copy link

codeclimate bot commented Mar 3, 2019

Code Climate has analyzed commit 2f564b9 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 89.4% (50% is the threshold).

This pull request will bring the total coverage in the repository to 91.4% (0.0% change).

View more on Code Climate.

@jpetermans
Copy link
Author

I agree with you on the server response, and script still should keep trying right up to when the check-in window opens. Hopefully this PR accomplishes that without hitting the server with a lot of true bad requests, just check-in attempts.

Two additional changes with this latest commit:

  • Script retries with get_checkin_data which also returns BAD_REQUEST if it hits the server a little early.
  • safe_requests tests for ERROR__AIR_TRAVEL__BEFORE_CHECKIN_WINDOW response from server which is when retries are needed.

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