Skip to content

Commit

Permalink
Rate limiter: clear up remaining todos
Browse files Browse the repository at this point in the history
  • Loading branch information
leviroth committed Jan 13, 2024
1 parent 4615b69 commit 55c9ccb
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 6 deletions.
8 changes: 4 additions & 4 deletions reddit_api_async/rate_limiter.ml
Original file line number Diff line number Diff line change
Expand Up @@ -230,8 +230,6 @@ module By_headers = struct
return (`Repeat ()))))
;;

(* TODO: Allow retries once per minute? *)

let update_server_side_info t ~new_server_side_info =
(match t.state with
| State.Created | State.Consuming_rate_limit _ -> ()
Expand All @@ -253,7 +251,10 @@ module By_headers = struct
let permit_request t =
let%bind () = wait_until_ready t in
match t.state with
| Waiting_on_first_request _ -> (* TODO is this impossible? *) assert false
| Waiting_on_first_request _ ->
raise_s
[%message
"Unexpectedly in [Waiting_on_first_request] state after [wait_until_ready]."]
| Created ->
let received_response = Ivar.create () in
t.state <- Waiting_on_first_request { received_response };
Expand All @@ -275,7 +276,6 @@ module By_headers = struct
(* We assume that, in the absence of ratelimit headers, we must have hit
some authentication failure. As a heuristic to avoid getting stuck, we
immediately reset [t.ready]. *)
(* TODO: What to do here? *)
t.state <- Created
| Some response_server_side_info ->
let new_server_side_info =
Expand Down
4 changes: 2 additions & 2 deletions test/test_rate_limiter.ml
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,11 @@ let%expect_test _ =
Rate_limiter.notify_response
rate_limiter
(build_header ~server_time:(00 ^: 10) ~limit_remaining:10);
let%bind () = Log.Global.flushed () in
print ();
[%expect
{|
("Rate limit is resetting"(old_remaining_api_calls 0))
((time (1970-01-01 00:00:00.000000000Z))
(is_ready true)
(rate_limiter (
Expand All @@ -138,10 +140,8 @@ let%expect_test _ =
return (`Repeat (n - 1)))
in
print ();
(* TODO It's not actually resetting: It's just exhausted. *)
[%expect
{|
("Rate limit is resetting"(old_remaining_api_calls 0))
((time (1970-01-01 00:00:00.000000000Z))
(is_ready false)
(rate_limiter (
Expand Down

0 comments on commit 55c9ccb

Please sign in to comment.