search

ash-project / ash

A declarative, extensible framework for building Elixir applications.
https://www.ash-hq.org
MIT License
1.57k stars 208 forks source link

Bulk actions ignore `around_action` hooks #1473

Closed vonagam closed 5 days ago

vonagam commented 5 days ago

Bulk actions currently silently ignore around_action hooks.

Also bulk actions do not add :action tracer spans.

(I've reported it before on Discord, repeating just to keep track of it.)

zachdaniel commented 5 days ago

I thought I fixed the around action issue. Can you confirm that's still broken in latest ash?

As for the other one, we probably need to add new span types for that, specifically "atomic_update" and "bulk_action_batch".

vonagam commented 5 days ago

I'm on 3.4.20 and based on what I see they do not run.

Looking at codebase I see that around hooks are executed in Ash.Changeset.with_hooks but that method is not used in bulk actions, only normal ones.

The only reference to around hooks in bulk is this check in bulk update. Am I missing something?

vonagam commented 5 days ago

Hm... so that check, as I understand, should force normal execution of an action if there are those hooks present.

But it applies only to bulk update, there is no such code for bulk create/destroy.

zachdaniel commented 5 days ago

Got it. We need it to apply to all three action types. Would you mind making a PR for that?