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

Hook system improvements #995

Open
endgame opened this issue Jun 10, 2024 · 1 comment
Open

Hook system improvements #995

endgame opened this issue Jun 10, 2024 · 1 comment
Milestone

Comments

@endgame
Copy link
Collaborator

endgame commented Jun 10, 2024

While writing an example that uses hooks, I noticed that they were very hard to use correctly, and reasonable-looking code would silently ignore the request type you think you're hooking into.

We should provide {add,remove}Hook{,_} variants which are specialised to each hook type and are harder to misuse:

  1. addHookFor and addHookFor_ should be moved to the bottom of the file and marked {-# DEPRECATED #-} with a message recommending the hook-specific functions, and promising removal in 2.2. The export list for Amazonka.Env.Hooks should have a -- * section for deprecated functions.
  2. addAWSRequestHook should be renamed to addRequestHookFor, and addAWSRequestHook should remain as a {-# DEPRECATED #-} alias, also scheduled for removal in 2.2.
  3. addAWSRequestHook_ should be deprecated and scheduled for removal in 2.2.
  4. removeHooksFor and removeHooksFor_ should be moved to the bottom of the file and deprecated, much the same as №1.
  5. For each field in the Hooks record (example requestRetry) that has a forall a, we should have:
    • An add-for function for this hook: addRequestRetryHookFor :: forall a b. Hook_ (Request a, Text, Retry.RetryStatus) -> Hook_ (Request b, Text, Retry.RetryStatus) -> Hook_ (Request b, Text, Retry.RetryStatus) = addHookFor_ @(Request a, Text, Retry.RetryStatus)
    • A remove-for function for this hook: removeRequestRetryHookFor :: _ = removeHookFor_
  6. Haddocks for each new function from №5
  7. Export these in a new -- ** Functions for specific hooks first, with an explanatory note:
    -- ** Functions for specific hooks
    -- $functions-specific-hooks (say that the type application for these is the AWS Request type e.g. `@S3.GetObject`.)
    addRequestHookFor,
    removeRequestsHooksFor,
    ...
    -- ** Functions for arbitrary hooks
    -- $functions-arbitrary-hooks
    noHook,
    noHook_,
    addHook,
    addHook_,
    ...
@endgame endgame added this to the 2.1 milestone Jun 10, 2024
@endgame
Copy link
Collaborator Author

endgame commented Jun 10, 2024

@dalpd as discussed at ZuriHac

dalpd added a commit to dalpd/amazonka that referenced this issue Jun 16, 2024
This pre-emptively deprecates the generic and easy to misuse hook helpers
`addHook` and `addHook_` before the introduction of the new variants.
Relates to brendanhay#995.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant