Add script for automatic submit of registry change requests#5058
Add script for automatic submit of registry change requests#5058IvanIvanoff merged 1 commit intomasterfrom
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 5 minutes and 8 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a new standalone Elixir script that finds unverified metrics whose names include "sentiment" and "ratio" with Changes
Sequence DiagramsequenceDiagram
actor Admin
participant Script as Sentiment Script
participant Registry as Metrics Registry
participant API as ChangeSuggestion API
participant Logger as Logger
Admin->>Script: Execute script
Script->>Registry: Query metrics (unverified, name contains "sentiment" & "ratio", aggregation == "sum")
Registry-->>Script: Return matching metrics
Script->>Logger: Print matched metrics
loop For each metric
Script->>API: Submit change suggestion (default_aggregation: "avg", note, submitter)
alt Success
API-->>Script: Created
Script->>Logger: Log success
else Error
API-->>Script: Error
Script->>Logger: Log error
end
end
Script->>Logger: Log summary of successes/failures
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
scripts/submit_sentiment_ratio_aggregation_change.exs (1)
37-50: UseTask.async_stream/3for metric processing.Line 38 currently processes all metrics sequentially with
Enum.map/2; the repository guidelines require using concurrent enumeration with back-pressure for this pattern.♻️ Proposed refactor
results = - Enum.map(metrics, fn metric -> - params = %{"default_aggregation" => "avg"} - - case ChangeSuggestion.create_change_suggestion(metric, params, notes, submitted_by) do - {:ok, suggestion} -> - IO.puts("[OK] Change request created for #{metric.metric} (suggestion id: #{suggestion.id})") - {:ok, metric.metric} - - {:error, error} -> - IO.puts("[ERROR] Failed to create change request for #{metric.metric}: #{inspect(error)}") - {:error, metric.metric, error} - end - end) + metrics + |> Task.async_stream( + fn metric -> + params = %{"default_aggregation" => "avg"} + + case ChangeSuggestion.create_change_suggestion(metric, params, notes, submitted_by) do + {:ok, suggestion} -> + IO.puts("[OK] Change request created for #{metric.metric} (suggestion id: #{suggestion.id})") + {:ok, metric.metric} + + {:error, error} -> + IO.puts("[ERROR] Failed to create change request for #{metric.metric}: #{inspect(error)}") + {:error, metric.metric, error} + end + end, + timeout: :infinity + ) + |> Enum.map(fn + {:ok, result} -> result + {:exit, reason} -> {:error, :task_exit, reason} + end)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/submit_sentiment_ratio_aggregation_change.exs` around lines 37 - 50, The current sequential Enum.map over metrics should be replaced with Task.async_stream/3 to process metrics concurrently with back-pressure: call Task.async_stream(metrics, fn metric -> ... end, max_concurrency: <sane_value>, timeout: :infinity) and move the existing body that calls ChangeSuggestion.create_change_suggestion(metric, params, notes, submitted_by) (including the IO.puts for success and error and the {:ok,...}/{:error,...} tuples) into the async function; finally collect the stream results with Enum.to_list() (or appropriate Enum functions) to produce the same results variable. Ensure you pass a reasonable max_concurrency (e.g., System.schedulers_online() or a configured constant) and keep the same tuple shapes returned by the worker function.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/submit_sentiment_ratio_aggregation_change.exs`:
- Around line 20-26: The Ecto query in the from(...) with alias Registry uses
m.default_aggregation == :sum (an atom) but the schema stores
default_aggregation as a string; change the predicate to compare against the
string "sum" so the filter matches actual rows—update the where clause that
references m.default_aggregation in the query (the from(m in Registry, ...
where: ... m.default_aggregation == :sum)) to use "sum" instead of :sum.
---
Nitpick comments:
In `@scripts/submit_sentiment_ratio_aggregation_change.exs`:
- Around line 37-50: The current sequential Enum.map over metrics should be
replaced with Task.async_stream/3 to process metrics concurrently with
back-pressure: call Task.async_stream(metrics, fn metric -> ... end,
max_concurrency: <sane_value>, timeout: :infinity) and move the existing body
that calls ChangeSuggestion.create_change_suggestion(metric, params, notes,
submitted_by) (including the IO.puts for success and error and the
{:ok,...}/{:error,...} tuples) into the async function; finally collect the
stream results with Enum.to_list() (or appropriate Enum functions) to produce
the same results variable. Ensure you pass a reasonable max_concurrency (e.g.,
System.schedulers_online() or a configured constant) and keep the same tuple
shapes returned by the worker function.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 299a4b05-d089-4f86-8f2b-5b9d1825a3b8
📒 Files selected for processing (1)
scripts/submit_sentiment_ratio_aggregation_change.exs
c86d32a to
656705b
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
scripts/submit_sentiment_ratio_aggregation_change.exs (1)
37-53: PreferTask.async_stream/3for the batch submission loop.Line 38 runs submissions serially;
Task.async_stream/3provides bounded concurrency and back-pressure for this type of batch operation.Proposed refactor
-results = - Enum.map(metrics, fn metric -> +results = + Task.async_stream( + metrics, + fn metric -> params = %{"default_aggregation" => "avg"} - case ChangeSuggestion.create_change_suggestion(metric, params, notes, submitted_by) do - {:ok, suggestion} -> - IO.puts( - "[OK] Change request created for #{metric.metric} (suggestion id: #{suggestion.id})" - ) + case ChangeSuggestion.create_change_suggestion(metric, params, notes, submitted_by) do + {:ok, suggestion} -> + IO.puts( + "[OK] Change request created for #{metric.metric} (suggestion id: #{suggestion.id})" + ) - {:ok, metric.metric} + {:ok, metric.metric} - {:error, error} -> - IO.puts("[ERROR] Failed to create change request for #{metric.metric}: #{inspect(error)}") - {:error, metric.metric, error} - end - end) + {:error, error} -> + IO.puts("[ERROR] Failed to create change request for #{metric.metric}: #{inspect(error)}") + {:error, metric.metric, error} + end + end, + max_concurrency: 8, + timeout: :infinity, + ordered: false + ) + |> Enum.map(fn + {:ok, result} -> result + {:exit, reason} -> {:error, :task_exit, reason} + end)As per coding guidelines, "Use
Task.async_stream(collection, callback, options)for concurrent enumeration with back-pressure. Most times you will want to passtimeout: :infinityas an option."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/submit_sentiment_ratio_aggregation_change.exs` around lines 37 - 53, The loop currently enumerates `metrics` serially with `Enum.map` and calls `ChangeSuggestion.create_change_suggestion/4`; replace it with `Task.async_stream(metrics, fn metric -> ... end, options)` to run submissions concurrently with back-pressure, using options like `timeout: :infinity` and a bounded `max_concurrency` (e.g., based on schedulers) so you still collect results into `results` as a list; preserve the same success and error tuple shapes and the `IO.puts` messages inside the task function and gather the stream results (e.g., via `Enum.to_list`) into `results`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/submit_sentiment_ratio_aggregation_change.exs`:
- Around line 12-13: Replace the hardcoded submitted_by assignment with code
that reads the submitter identity from an environment variable or CLI arg (e.g.,
System.get_env("SUBMITTER_EMAIL") or OptionParser) and perform fail-fast
validation (ensure non-empty and basic email format) before proceeding; update
any uses of the submitted_by variable accordingly so the script fails with a
clear error when the env/arg is missing or invalid rather than storing a
personal email in source.
- Around line 37-53: The script creates duplicate ChangeSuggestion records on
reruns because there is no idempotency guard; update
ChangeSuggestion.create_change_suggestion (or the caller in the script) to first
query the DB for existing suggestions matching the same metric_registry_id and a
non-terminal status (e.g., :pending or :approved) and return/skip if found, or
add a DB-level unique constraint on the combination of metric_registry_id and
change payload to prevent duplicates; specifically, add a pre-check using the
same query logic before calling do_create (and ensure do_create also respects
the uniqueness check/returns a conflict rather than inserting blindly).
---
Nitpick comments:
In `@scripts/submit_sentiment_ratio_aggregation_change.exs`:
- Around line 37-53: The loop currently enumerates `metrics` serially with
`Enum.map` and calls `ChangeSuggestion.create_change_suggestion/4`; replace it
with `Task.async_stream(metrics, fn metric -> ... end, options)` to run
submissions concurrently with back-pressure, using options like `timeout:
:infinity` and a bounded `max_concurrency` (e.g., based on schedulers) so you
still collect results into `results` as a list; preserve the same success and
error tuple shapes and the `IO.puts` messages inside the task function and
gather the stream results (e.g., via `Enum.to_list`) into `results`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4074710e-53ba-4d58-a02b-8c78790a5dc3
📒 Files selected for processing (1)
scripts/submit_sentiment_ratio_aggregation_change.exs
656705b to
993afab
Compare
Changes
Ticket
Checklist:
Summary by CodeRabbit