You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "moliyadhaval (via GitHub)" <gi...@apache.org> on 2026/09/19 05:08:02 UTC
[PR] Pr 205 [superset]
moliyadhaval opened a new pull request, #44439:
URL: https://github.com/apache/superset/pull/44439
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] Pr 205 [superset]
Posted by "hainenber (via GitHub)" <gi...@apache.org>.
hainenber closed pull request #44439: Pr 205
URL: https://github.com/apache/superset/pull/44439
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] Pr 205 [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #44439:
URL: https://github.com/apache/superset/pull/44439#discussion_r4052373997
##########
automation/orchestrator/api.py:
##########
@@ -0,0 +1,342 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""HTTP clients for GitHub and the Devin API with a shared retry policy.
+
+Retry contract
+--------------
+* ``429`` and rate-limited ``403`` (primary/secondary/abuse) are retried for
+ every method: the server rejected the request, nothing happened.
+* ``5xx`` and transport errors (timeouts, resets) are retried for idempotent
+ methods (GET/HEAD/PUT/DELETE). For POST/PATCH they raise
+ :class:`AmbiguousWriteError` so the caller probes for the resource before
+ writing again -- this is what keeps retries idempotent.
+* Other ``4xx`` raise :class:`PermanentError` immediately.
+* Waits use exponential backoff with full jitter (tenacity), capped per
+ attempt and by a total budget, and honour ``Retry-After`` /
+ ``X-RateLimit-Reset`` when present.
+
+Secrets are only ever placed in request headers; they are never logged.
+"""
+
+from __future__ import annotations
+
+import email.utils
+import logging
+import os
+import time
+from collections.abc import Callable, Iterator
+from dataclasses import dataclass, field
+from typing import Any
+
+import requests
+from tenacity import (
+ retry_if_exception_type,
+ RetryCallState,
+ Retrying,
+ stop_after_attempt,
+ stop_after_delay,
+ wait_exponential_jitter,
+)
+
+log = logging.getLogger("automation.api")
+
+IDEMPOTENT_METHODS = frozenset({"GET", "PUT", "DELETE"})
+RATE_LIMIT_MARKERS = ("rate limit", "abuse detection", "too many requests")
+
+
+class ApiError(Exception):
+ """Base class for API failures."""
+
+ def __init__(self, message: str, status: int | None = None) -> None:
+ super().__init__(message)
+ self.status = status
+
+
+class TransientError(ApiError):
+ """Safe to retry: the request was rejected or the read failed."""
+
+ def __init__(
+ self, message: str, status: int | None = None, retry_after: float | None = None
+ ) -> None:
+ super().__init__(message, status)
+ self.retry_after = retry_after
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing docstrings on new __init__ methods</b></div>
<div id="fix">
BITO rule 12147 requires docstrings on every new function; `ApiError.__init__` and `TransientError.__init__` have none, and `TransientError.retry_after`'s semantics (seconds, server hint consumed by `_wait`) are only inferable from `_retry_after_seconds`. Add brief docstrings documenting the `status`/`retry_after` parameters.
</div>
</div>
<small><i>Code Review Run #8b0af2</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
automation/orchestrator/api.py:
##########
@@ -0,0 +1,342 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""HTTP clients for GitHub and the Devin API with a shared retry policy.
+
+Retry contract
+--------------
+* ``429`` and rate-limited ``403`` (primary/secondary/abuse) are retried for
+ every method: the server rejected the request, nothing happened.
+* ``5xx`` and transport errors (timeouts, resets) are retried for idempotent
+ methods (GET/HEAD/PUT/DELETE). For POST/PATCH they raise
+ :class:`AmbiguousWriteError` so the caller probes for the resource before
+ writing again -- this is what keeps retries idempotent.
+* Other ``4xx`` raise :class:`PermanentError` immediately.
+* Waits use exponential backoff with full jitter (tenacity), capped per
+ attempt and by a total budget, and honour ``Retry-After`` /
+ ``X-RateLimit-Reset`` when present.
+
+Secrets are only ever placed in request headers; they are never logged.
+"""
+
+from __future__ import annotations
+
+import email.utils
+import logging
+import os
+import time
+from collections.abc import Callable, Iterator
+from dataclasses import dataclass, field
+from typing import Any
+
+import requests
+from tenacity import (
+ retry_if_exception_type,
+ RetryCallState,
+ Retrying,
+ stop_after_attempt,
+ stop_after_delay,
+ wait_exponential_jitter,
+)
+
+log = logging.getLogger("automation.api")
+
+IDEMPOTENT_METHODS = frozenset({"GET", "PUT", "DELETE"})
+RATE_LIMIT_MARKERS = ("rate limit", "abuse detection", "too many requests")
+
+
+class ApiError(Exception):
+ """Base class for API failures."""
+
+ def __init__(self, message: str, status: int | None = None) -> None:
+ super().__init__(message)
+ self.status = status
+
+
+class TransientError(ApiError):
+ """Safe to retry: the request was rejected or the read failed."""
+
+ def __init__(
+ self, message: str, status: int | None = None, retry_after: float | None = None
+ ) -> None:
+ super().__init__(message, status)
+ self.retry_after = retry_after
+
+
+class AmbiguousWriteError(ApiError):
+ """A write may or may not have landed; probe before repeating it."""
+
+
+class PermanentError(ApiError):
+ """Client error that will not succeed on retry (401/404/422...)."""
+
+
+@dataclass(frozen=True)
+class RetryPolicy:
+ max_attempts: int = int(os.environ.get("RETRY_MAX_ATTEMPTS", "6"))
+ initial: float = float(os.environ.get("RETRY_BASE_SECONDS", "2"))
+ max_sleep: float = float(os.environ.get("RETRY_MAX_SLEEP", "120"))
+ budget_seconds: float = float(os.environ.get("RETRY_BUDGET_SECONDS", "900"))
+ timeout: float = float(os.environ.get("HTTP_TIMEOUT", "60"))
+
+
+def _wait(policy: RetryPolicy) -> Callable[[RetryCallState], float]:
+ """Exponential backoff with full jitter, overridden by a server hint."""
+ jitter = wait_exponential_jitter(initial=policy.initial, max=policy.max_sleep)
+
+ def wait(state: RetryCallState) -> float:
+ exc = state.outcome.exception() if state.outcome else None
+ if isinstance(exc, TransientError) and exc.retry_after is not None:
+ return min(max(exc.retry_after, 0.0), policy.max_sleep)
+ return float(jitter(state))
+
+ return wait
+
+
+def _retry_after_seconds(headers: Any) -> float | None:
+ """Parse Retry-After (seconds or HTTP-date) or X-RateLimit-Reset (epoch)."""
+ if raw := headers.get("Retry-After"):
+ try:
+ return float(raw)
+ except ValueError:
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Uncaught Retry-After parse error</b></div>
<div id="fix">
`_retry_after_seconds` guards `float(raw)` but not `email.utils.parsedate_to_datetime(raw)`: a malformed `Retry-After` (e.g. "soon") raises `ValueError` inside `_classify`'s raise path, escaping the `TransientError` contract and surfacing as an unhandled `ValueError` to `request()` callers. Catch the parse failure and return `None`.
</div>
</div>
<small><i>Code Review Run #8b0af2</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
automation/orchestrator/api.py:
##########
@@ -0,0 +1,342 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""HTTP clients for GitHub and the Devin API with a shared retry policy.
+
+Retry contract
+--------------
+* ``429`` and rate-limited ``403`` (primary/secondary/abuse) are retried for
+ every method: the server rejected the request, nothing happened.
+* ``5xx`` and transport errors (timeouts, resets) are retried for idempotent
+ methods (GET/HEAD/PUT/DELETE). For POST/PATCH they raise
+ :class:`AmbiguousWriteError` so the caller probes for the resource before
+ writing again -- this is what keeps retries idempotent.
+* Other ``4xx`` raise :class:`PermanentError` immediately.
+* Waits use exponential backoff with full jitter (tenacity), capped per
+ attempt and by a total budget, and honour ``Retry-After`` /
+ ``X-RateLimit-Reset`` when present.
+
+Secrets are only ever placed in request headers; they are never logged.
+"""
+
+from __future__ import annotations
+
+import email.utils
+import logging
+import os
+import time
+from collections.abc import Callable, Iterator
+from dataclasses import dataclass, field
+from typing import Any
+
+import requests
+from tenacity import (
+ retry_if_exception_type,
+ RetryCallState,
+ Retrying,
+ stop_after_attempt,
+ stop_after_delay,
+ wait_exponential_jitter,
+)
+
+log = logging.getLogger("automation.api")
+
+IDEMPOTENT_METHODS = frozenset({"GET", "PUT", "DELETE"})
+RATE_LIMIT_MARKERS = ("rate limit", "abuse detection", "too many requests")
+
+
+class ApiError(Exception):
+ """Base class for API failures."""
+
+ def __init__(self, message: str, status: int | None = None) -> None:
+ super().__init__(message)
+ self.status = status
+
+
+class TransientError(ApiError):
+ """Safe to retry: the request was rejected or the read failed."""
+
+ def __init__(
+ self, message: str, status: int | None = None, retry_after: float | None = None
+ ) -> None:
+ super().__init__(message, status)
+ self.retry_after = retry_after
+
+
+class AmbiguousWriteError(ApiError):
+ """A write may or may not have landed; probe before repeating it."""
+
+
+class PermanentError(ApiError):
+ """Client error that will not succeed on retry (401/404/422...)."""
+
+
+@dataclass(frozen=True)
+class RetryPolicy:
+ max_attempts: int = int(os.environ.get("RETRY_MAX_ATTEMPTS", "6"))
+ initial: float = float(os.environ.get("RETRY_BASE_SECONDS", "2"))
+ max_sleep: float = float(os.environ.get("RETRY_MAX_SLEEP", "120"))
+ budget_seconds: float = float(os.environ.get("RETRY_BUDGET_SECONDS", "900"))
+ timeout: float = float(os.environ.get("HTTP_TIMEOUT", "60"))
+
+
+def _wait(policy: RetryPolicy) -> Callable[[RetryCallState], float]:
+ """Exponential backoff with full jitter, overridden by a server hint."""
+ jitter = wait_exponential_jitter(initial=policy.initial, max=policy.max_sleep)
+
+ def wait(state: RetryCallState) -> float:
+ exc = state.outcome.exception() if state.outcome else None
+ if isinstance(exc, TransientError) and exc.retry_after is not None:
+ return min(max(exc.retry_after, 0.0), policy.max_sleep)
+ return float(jitter(state))
+
+ return wait
+
+
+def _retry_after_seconds(headers: Any) -> float | None:
+ """Parse Retry-After (seconds or HTTP-date) or X-RateLimit-Reset (epoch)."""
+ if raw := headers.get("Retry-After"):
+ try:
+ return float(raw)
+ except ValueError:
+ parsed = email.utils.parsedate_to_datetime(raw)
+ return max(parsed.timestamp() - time.time(), 0.0)
+ reset = headers.get("X-RateLimit-Reset")
+ remaining = headers.get("X-RateLimit-Remaining")
+ if reset and remaining == "0":
+ try:
+ return max(float(reset) - time.time(), 0.0)
+ except ValueError:
+ return None
+ return None
+
+
+def _log_retry(policy: RetryPolicy) -> Callable[[RetryCallState], None]:
+ def before_sleep(state: RetryCallState) -> None:
+ log.warning(
+ "%s. Attempt %d/%d failed, retrying in %.1fs",
+ state.outcome.exception() if state.outcome else "",
+ state.attempt_number,
+ policy.max_attempts,
+ state.next_action.sleep if state.next_action else 0.0,
+ )
+
+ return before_sleep
+
+
+@dataclass
+class BaseClient:
+ """A ``requests`` session with the shared retry policy applied."""
+
+ base_url: str
+ token_env: str
+ policy: RetryPolicy = field(default_factory=RetryPolicy)
+ user_agent: str = "superset-automation-orchestrator"
+ session: requests.Session = field(default_factory=requests.Session)
+
+ def _headers(self) -> dict[str, str]:
+ headers = {"User-Agent": self.user_agent, "Accept": "application/json"}
+ if token := os.environ.get(self.token_env):
+ headers["Authorization"] = f"Bearer {token}"
+ return headers
+
+ def _classify(self, resp: requests.Response, method: str, context: str) -> None:
+ status = resp.status_code
+ if status < 400:
+ return
+ body = resp.text[:300]
+ snippet = f"HTTP {status} {resp.reason}: {body}"
+ if status == 429 or (
+ status == 403
+ and (
+ resp.headers.get("X-RateLimit-Remaining") == "0"
+ or any(m in body.lower() for m in RATE_LIMIT_MARKERS)
+ )
+ ):
+ raise TransientError(
+ f"{context} rate limited ({snippet})",
+ status,
+ _retry_after_seconds(resp.headers),
+ )
+ if status >= 500:
+ if method in IDEMPOTENT_METHODS:
+ raise TransientError(f"{context} {snippet}", status)
+ raise AmbiguousWriteError(f"{context} {snippet}", status)
+ raise PermanentError(f"{context} {snippet}", status)
+
+ def _once(
+ self,
+ method: str,
+ path: str,
+ context: str,
+ params: dict[str, Any] | None,
+ json_body: Any,
+ ) -> requests.Response:
+ url = path if path.startswith("http") else f"{self.base_url}/{path.lstrip('/')}"
+ try:
+ resp = self.session.request(
+ method,
+ url,
+ headers=self._headers(),
+ params=params,
+ json=json_body,
+ timeout=self.policy.timeout,
+ )
+ except (requests.ConnectionError, requests.Timeout) as exc:
+ reason = f"{context} {type(exc).__name__}"
+ if method in IDEMPOTENT_METHODS:
+ raise TransientError(reason) from exc
+ raise AmbiguousWriteError(reason) from exc
+ self._classify(resp, method, context)
+ return resp
+
+ def request(
+ self,
+ method: str,
+ path: str,
+ *,
+ context: str = "",
+ params: dict[str, Any] | None = None,
+ json_body: Any = None,
+ ) -> requests.Response:
+ """Perform one logical request under the retry contract."""
+ method = method.upper()
+ context = context or f"{method} {path}"
+ retrying = Retrying(
+ retry=retry_if_exception_type(TransientError),
+ wait=_wait(self.policy),
+ stop=(
+ stop_after_attempt(self.policy.max_attempts)
+ | stop_after_delay(self.policy.budget_seconds)
+ ),
+ before_sleep=_log_retry(self.policy),
+ reraise=True,
+ )
+ return retrying(self._once, method, path, context, params, json_body)
+
+ def get_json(self, path: str, **kwargs: Any) -> Any:
+ return self.request("GET", path, **kwargs).json()
+
+
+class GitHub(BaseClient):
+ """GitHub REST client bound to one repository."""
+
+ def __init__(self, repo: str, policy: RetryPolicy | None = None) -> None:
+ super().__init__(
+ base_url=os.environ.get("GITHUB_API", "https://api.github.com"),
+ token_env="GH_TOKEN", # noqa: S106 -- env var name, not a secret
+ policy=policy or RetryPolicy(),
+ )
+ self.repo = repo
+
+ def _headers(self) -> dict[str, str]:
+ headers = super()._headers()
+ headers["X-GitHub-Api-Version"] = "2022-11-28"
+ headers["Accept"] = "application/vnd.github+json"
+ return headers
+
+ def paginate(self, path: str, **params: Any) -> Iterator[dict[str, Any]]:
+ params.setdefault("per_page", 100)
+ url: str | None = f"repos/{self.repo}/{path}"
+ while url:
+ resp = self.request("GET", url, params=params)
+ yield from resp.json()
+ url = resp.links.get("next", {}).get("url")
+ params = {}
+
+ def open_issues(self, label: str) -> list[dict[str, Any]]:
+ return [
+ i
+ for i in self.paginate("issues", state="open", labels=label)
+ if "pull_request" not in i
+ ]
+
+ def pulls(self, state: str = "all") -> list[dict[str, Any]]:
+ return list(self.paginate("pulls", state=state))
+
+ def branch_commit_date(self, name: str) -> str:
+ data = self.get_json(f"repos/{self.repo}/branches/{name}")
+ return str(data["commit"]["commit"]["committer"]["date"])
+
+ def delete_branch(self, name: str) -> None:
+ self.request(
+ "DELETE",
+ f"repos/{self.repo}/git/refs/heads/{name}",
+ context=f"delete branch {name}",
+ )
+
+ def comments(self, issue: int) -> list[dict[str, Any]]:
+ return list(self.paginate(f"issues/{issue}/comments"))
+
+ def create_comment(self, issue: int, body: str) -> dict[str, Any]:
+ return self.request(
+ "POST",
+ f"repos/{self.repo}/issues/{issue}/comments",
+ context=f"[Issue #{issue}] create comment",
+ json_body={"body": body},
+ ).json()
+
+ def update_comment(self, comment_id: int, body: str) -> dict[str, Any]:
+ return self.request(
+ "PATCH",
+ f"repos/{self.repo}/issues/comments/{comment_id}",
+ context=f"update comment {comment_id}",
+ json_body={"body": body},
+ ).json()
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Empty-repo 404 aborts sweep</b></div>
<div id="fix">
`paginate` treats every non-2xx as an error, but GitHub returns 404 for `/branches` on an empty repository, so `branch_for_issue`/`stale_branches` in `dispatch.py` abort with `PermanentError` instead of seeing zero branches. Consider treating a first-page 404 as an empty result, or handling it at those call sites.
</div>
</div>
<div id="suggestion">
<div id="issue"><b>Missing method docstrings</b></div>
<div id="fix">
Per BITO adaptive rule 12147, every newly introduced Python function needs a docstring; the eight methods added here (`paginate` through `update_comment`) have none. Brief docstrings would document pagination semantics (e.g. that `params` only applies to the first request) without reading callers.
</div>
</div>
<small><i>Code Review Run #8b0af2</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
automation/orchestrator/api.py:
##########
@@ -0,0 +1,342 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""HTTP clients for GitHub and the Devin API with a shared retry policy.
+
+Retry contract
+--------------
+* ``429`` and rate-limited ``403`` (primary/secondary/abuse) are retried for
+ every method: the server rejected the request, nothing happened.
+* ``5xx`` and transport errors (timeouts, resets) are retried for idempotent
+ methods (GET/HEAD/PUT/DELETE). For POST/PATCH they raise
+ :class:`AmbiguousWriteError` so the caller probes for the resource before
+ writing again -- this is what keeps retries idempotent.
+* Other ``4xx`` raise :class:`PermanentError` immediately.
+* Waits use exponential backoff with full jitter (tenacity), capped per
+ attempt and by a total budget, and honour ``Retry-After`` /
+ ``X-RateLimit-Reset`` when present.
+
+Secrets are only ever placed in request headers; they are never logged.
+"""
+
+from __future__ import annotations
+
+import email.utils
+import logging
+import os
+import time
+from collections.abc import Callable, Iterator
+from dataclasses import dataclass, field
+from typing import Any
+
+import requests
+from tenacity import (
+ retry_if_exception_type,
+ RetryCallState,
+ Retrying,
+ stop_after_attempt,
+ stop_after_delay,
+ wait_exponential_jitter,
+)
+
+log = logging.getLogger("automation.api")
+
+IDEMPOTENT_METHODS = frozenset({"GET", "PUT", "DELETE"})
+RATE_LIMIT_MARKERS = ("rate limit", "abuse detection", "too many requests")
+
+
+class ApiError(Exception):
+ """Base class for API failures."""
+
+ def __init__(self, message: str, status: int | None = None) -> None:
+ super().__init__(message)
+ self.status = status
+
+
+class TransientError(ApiError):
+ """Safe to retry: the request was rejected or the read failed."""
+
+ def __init__(
+ self, message: str, status: int | None = None, retry_after: float | None = None
+ ) -> None:
+ super().__init__(message, status)
+ self.retry_after = retry_after
+
+
+class AmbiguousWriteError(ApiError):
+ """A write may or may not have landed; probe before repeating it."""
+
+
+class PermanentError(ApiError):
+ """Client error that will not succeed on retry (401/404/422...)."""
+
+
+@dataclass(frozen=True)
+class RetryPolicy:
+ max_attempts: int = int(os.environ.get("RETRY_MAX_ATTEMPTS", "6"))
+ initial: float = float(os.environ.get("RETRY_BASE_SECONDS", "2"))
+ max_sleep: float = float(os.environ.get("RETRY_MAX_SLEEP", "120"))
+ budget_seconds: float = float(os.environ.get("RETRY_BUDGET_SECONDS", "900"))
+ timeout: float = float(os.environ.get("HTTP_TIMEOUT", "60"))
+
+
+def _wait(policy: RetryPolicy) -> Callable[[RetryCallState], float]:
+ """Exponential backoff with full jitter, overridden by a server hint."""
+ jitter = wait_exponential_jitter(initial=policy.initial, max=policy.max_sleep)
+
+ def wait(state: RetryCallState) -> float:
+ exc = state.outcome.exception() if state.outcome else None
+ if isinstance(exc, TransientError) and exc.retry_after is not None:
+ return min(max(exc.retry_after, 0.0), policy.max_sleep)
+ return float(jitter(state))
+
+ return wait
+
+
+def _retry_after_seconds(headers: Any) -> float | None:
+ """Parse Retry-After (seconds or HTTP-date) or X-RateLimit-Reset (epoch)."""
+ if raw := headers.get("Retry-After"):
+ try:
+ return float(raw)
+ except ValueError:
+ parsed = email.utils.parsedate_to_datetime(raw)
+ return max(parsed.timestamp() - time.time(), 0.0)
+ reset = headers.get("X-RateLimit-Reset")
+ remaining = headers.get("X-RateLimit-Remaining")
+ if reset and remaining == "0":
+ try:
+ return max(float(reset) - time.time(), 0.0)
+ except ValueError:
+ return None
+ return None
+
+
+def _log_retry(policy: RetryPolicy) -> Callable[[RetryCallState], None]:
+ def before_sleep(state: RetryCallState) -> None:
+ log.warning(
+ "%s. Attempt %d/%d failed, retrying in %.1fs",
+ state.outcome.exception() if state.outcome else "",
+ state.attempt_number,
+ policy.max_attempts,
+ state.next_action.sleep if state.next_action else 0.0,
+ )
+
+ return before_sleep
+
+
+@dataclass
+class BaseClient:
+ """A ``requests`` session with the shared retry policy applied."""
+
+ base_url: str
+ token_env: str
+ policy: RetryPolicy = field(default_factory=RetryPolicy)
+ user_agent: str = "superset-automation-orchestrator"
+ session: requests.Session = field(default_factory=requests.Session)
+
+ def _headers(self) -> dict[str, str]:
+ headers = {"User-Agent": self.user_agent, "Accept": "application/json"}
+ if token := os.environ.get(self.token_env):
+ headers["Authorization"] = f"Bearer {token}"
+ return headers
+
+ def _classify(self, resp: requests.Response, method: str, context: str) -> None:
+ status = resp.status_code
+ if status < 400:
+ return
+ body = resp.text[:300]
+ snippet = f"HTTP {status} {resp.reason}: {body}"
+ if status == 429 or (
+ status == 403
+ and (
+ resp.headers.get("X-RateLimit-Remaining") == "0"
+ or any(m in body.lower() for m in RATE_LIMIT_MARKERS)
+ )
+ ):
+ raise TransientError(
+ f"{context} rate limited ({snippet})",
+ status,
+ _retry_after_seconds(resp.headers),
+ )
+ if status >= 500:
+ if method in IDEMPOTENT_METHODS:
+ raise TransientError(f"{context} {snippet}", status)
+ raise AmbiguousWriteError(f"{context} {snippet}", status)
+ raise PermanentError(f"{context} {snippet}", status)
+
+ def _once(
+ self,
+ method: str,
+ path: str,
+ context: str,
+ params: dict[str, Any] | None,
+ json_body: Any,
+ ) -> requests.Response:
+ url = path if path.startswith("http") else f"{self.base_url}/{path.lstrip('/')}"
+ try:
+ resp = self.session.request(
+ method,
+ url,
+ headers=self._headers(),
+ params=params,
+ json=json_body,
+ timeout=self.policy.timeout,
+ )
+ except (requests.ConnectionError, requests.Timeout) as exc:
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missed transport exceptions</b></div>
<div id="fix">
`_once` catches only `requests.ConnectionError`/`Timeout`, but `Session.send` reads `r.content` for non-stream requests, so a truncated chunked body raises `requests.ChunkedEncodingError` โ a direct `RequestException` subclass, not a `ConnectionError` (verified against requests 2.33.0). It escapes unclassified, bypassing the module's documented retry contract. Add the decoding-error types to the except tuple.
</div>
</div>
<small><i>Code Review Run #8b0af2</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
automation/orchestrator/api.py:
##########
@@ -0,0 +1,342 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""HTTP clients for GitHub and the Devin API with a shared retry policy.
+
+Retry contract
+--------------
+* ``429`` and rate-limited ``403`` (primary/secondary/abuse) are retried for
+ every method: the server rejected the request, nothing happened.
+* ``5xx`` and transport errors (timeouts, resets) are retried for idempotent
+ methods (GET/HEAD/PUT/DELETE). For POST/PATCH they raise
+ :class:`AmbiguousWriteError` so the caller probes for the resource before
+ writing again -- this is what keeps retries idempotent.
+* Other ``4xx`` raise :class:`PermanentError` immediately.
+* Waits use exponential backoff with full jitter (tenacity), capped per
+ attempt and by a total budget, and honour ``Retry-After`` /
+ ``X-RateLimit-Reset`` when present.
+
+Secrets are only ever placed in request headers; they are never logged.
+"""
+
+from __future__ import annotations
+
+import email.utils
+import logging
+import os
+import time
+from collections.abc import Callable, Iterator
+from dataclasses import dataclass, field
+from typing import Any
+
+import requests
+from tenacity import (
+ retry_if_exception_type,
+ RetryCallState,
+ Retrying,
+ stop_after_attempt,
+ stop_after_delay,
+ wait_exponential_jitter,
+)
+
+log = logging.getLogger("automation.api")
+
+IDEMPOTENT_METHODS = frozenset({"GET", "PUT", "DELETE"})
+RATE_LIMIT_MARKERS = ("rate limit", "abuse detection", "too many requests")
+
+
+class ApiError(Exception):
+ """Base class for API failures."""
+
+ def __init__(self, message: str, status: int | None = None) -> None:
+ super().__init__(message)
+ self.status = status
+
+
+class TransientError(ApiError):
+ """Safe to retry: the request was rejected or the read failed."""
+
+ def __init__(
+ self, message: str, status: int | None = None, retry_after: float | None = None
+ ) -> None:
+ super().__init__(message, status)
+ self.retry_after = retry_after
+
+
+class AmbiguousWriteError(ApiError):
+ """A write may or may not have landed; probe before repeating it."""
+
+
+class PermanentError(ApiError):
+ """Client error that will not succeed on retry (401/404/422...)."""
+
+
+@dataclass(frozen=True)
+class RetryPolicy:
+ max_attempts: int = int(os.environ.get("RETRY_MAX_ATTEMPTS", "6"))
+ initial: float = float(os.environ.get("RETRY_BASE_SECONDS", "2"))
+ max_sleep: float = float(os.environ.get("RETRY_MAX_SLEEP", "120"))
+ budget_seconds: float = float(os.environ.get("RETRY_BUDGET_SECONDS", "900"))
+ timeout: float = float(os.environ.get("HTTP_TIMEOUT", "60"))
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Env parsed at import in dataclass defaults</b></div>
<div id="fix">
`RetryPolicy` field defaults call `os.environ.get(...)` in the class body, so values are captured once at import: later env changes are ignored (every `RetryPolicy()` reuses the snapshot, e.g. `GitHub`/`Devin` defaults), and a malformed value (e.g. RETRY_MAX_ATTEMPTS=abc) raises ValueError at import, crashing the process before any logging. Parse env in a `from_env()` factory instead.
</div>
</div>
<small><i>Code Review Run #8b0af2</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
automation/orchestrator/dispatch.py:
##########
@@ -0,0 +1,290 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""Dispatch one Devin fix session per open nightly-scan issue, safely.
+
+Every step is idempotent: a session is looked up by title before it is
+created, a dashboard comment is looked up by a hidden marker before it is
+posted, and a failure for one issue is recorded and the loop moves on.
+"""
+
+from __future__ import annotations
+
+import logging
+import re
+import time
+from collections.abc import Callable, Iterable
+from dataclasses import dataclass
+from datetime import datetime, timedelta, timezone
+from typing import Any
+
+from .api import AmbiguousWriteError, ApiError, Devin, GitHub, session_url
+
+log = logging.getLogger("automation.dispatch")
+
+BRANCH_PREFIX = "devin/nightly-fix-"
+BRANCH_ISSUE = re.compile(r"devin/nightly-fix-(?:issue-)?(\d+)")
+BODY_REF = re.compile(r"\b(?:Fixes|Refs|Closes)\s+#(\d+)", re.I)
+DEAD_STATUSES = frozenset({"error", "terminated", "cancelled", "canceled"})
+FINISHED_STATUSES = frozenset({"finished", "completed", "blocked", "suspended"})
+
+
+@dataclass
+class Outcome:
+ issue: int
+ status: str # dispatched | skipped | failed
+ detail: str = ""
+
+
+def marker(key: str) -> str:
+ return f"<!-- devin:{key} -->"
+
+
+def attempted_issues(
+ prs: Iterable[dict[str, Any]], sessions: Iterable[dict[str, Any]]
+) -> set[int]:
+ """Issue numbers that already have a PR, a fix branch, or a live session."""
+ seen: set[int] = set()
+ for pr in prs:
+ seen.update(int(n) for n in BODY_REF.findall(pr.get("body") or ""))
+ if m := BRANCH_ISSUE.search(pr["head"]["ref"]):
+ seen.add(int(m.group(1)))
+ for s in sessions:
+ if str(s.get("status") or s.get("status_enum") or "").lower() in DEAD_STATUSES:
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>status logic duplicated</b></div>
<div id="fix">
The expression `str(s.get("status") or s.get("status_enum") or "").lower()` is duplicated verbatim at lines 64, 82-83, and 130. A shared `session_status(s)` helper would centralize the status normalization and prevent the three sites from drifting (e.g. one adding a new status key).
</div>
</div>
<small><i>Code Review Run #8b0af2</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
automation/tests/test_orchestrator.py:
##########
@@ -0,0 +1,243 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""Behavioural tests for the orchestrator against a fake GitHub/Devin API.
+
+Run with ``python -m pytest automation/tests``.
+"""
+
+from __future__ import annotations
+
+import json # noqa: TID251 -- standalone tool, not part of the superset package
+import threading
+from collections.abc import Iterator
+from datetime import timedelta
+from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
+from typing import Any
+
+import pytest
+
+from automation.orchestrator import dispatch as ops
+from automation.orchestrator.api import (
+ AmbiguousWriteError,
+ Devin,
+ GitHub,
+ PermanentError,
+ RetryPolicy,
+ TransientError,
+)
+
+FAST = RetryPolicy(
+ max_attempts=4, initial=0.01, max_sleep=0.05, budget_seconds=5, timeout=1
+)
+
+
+class Fake:
+ """Scriptable API: ``script[path]`` is a list of (status, body) replies."""
+
+ def __init__(self) -> None:
+ self.script: dict[str, list[tuple[int, Any]]] = {}
+ self.calls: list[tuple[str, str]] = []
+ self.comments: list[dict[str, Any]] = []
+ self.sessions: list[dict[str, Any]] = []
+
+ def reply(self, method: str, path: str) -> tuple[int, Any, dict[str, str]]:
+ self.calls.append((method, path))
+ if path in self.script and self.script[path]:
+ status, body = self.script[path].pop(0)
+ headers = {"Retry-After": "0"} if status == 429 else {}
+ return status, body, headers
+ if path.endswith("/comments") and method == "GET":
+ return 200, self.comments, {}
+ if path.endswith("/comments") and method == "POST":
+ return 500, {"message": "boom"}, {} # landed, but reply lost
+ if path.startswith("/sessions") and method == "GET":
+ return 200, {"items": self.sessions, "has_next_page": False}, {}
+ if path == "/sessions" and method == "POST":
+ return 500, {"message": "boom"}, {}
+ return 200, [], {}
+
+
+@pytest.fixture
+def fake() -> Iterator[tuple[Fake, str]]:
+ state = Fake()
+
+ class Handler(BaseHTTPRequestHandler):
+ def _handle(self) -> None:
+ length = int(self.headers.get("Content-Length") or 0)
+ payload = json.loads(self.rfile.read(length) or b"{}") if length else {}
+ path = self.path.split("?")[0]
+ if path.endswith("/comments") and self.command == "POST":
+ state.comments.append(
+ {"id": len(state.comments) + 1, "body": payload["body"]}
+ )
+ if path == "/sessions" and self.command == "POST":
+ state.sessions.append(
+ {"session_id": "devin-1", "status": "running", **payload}
+ )
+ status, body, headers = state.reply(self.command, path)
+ data = json.dumps(body).encode()
+ self.send_response(status)
+ self.send_header("Content-Type", "application/json")
+ for k, v in headers.items():
+ self.send_header(k, v)
+ self.end_headers()
+ self.wfile.write(data)
+
+ do_GET = do_POST = do_PATCH = do_DELETE = _handle # noqa: N815
+
+ def log_message(self, *_: Any) -> None:
+ pass
+
+ server = ThreadingHTTPServer(("127.0.0.1", 0), Handler)
+ threading.Thread(target=server.serve_forever, daemon=True).start()
+ try:
+ yield state, f"http://127.0.0.1:{server.server_port}"
+ finally:
+ server.shutdown()
+
+
+def gh_client(base: str) -> GitHub:
+ gh = GitHub("o/r", FAST)
+ gh.base_url = base
+ return gh
+
+
+def devin_client(base: str) -> Devin:
+ d = Devin("org", FAST)
+ d.base_url = base
+ return d
+
+
+def test_get_retries_429_and_500_then_succeeds(fake: tuple[Fake, str]) -> None:
+ state, base = fake
+ state.script["/x"] = [(429, {}), (500, {}), (200, {"ok": True})]
+ assert gh_client(base).get_json("x") == {"ok": True}
+ assert [c for c in state.calls if c[1] == "/x"] == [("GET", "/x")] * 3
+
+
+def test_rate_limited_403_is_transient(fake: tuple[Fake, str]) -> None:
+ state, base = fake
+ state.script["/x"] = [(403, {"message": "API rate limit exceeded"}), (200, 1)]
+ assert gh_client(base).get_json("x") == 1
+
+
+def test_404_fails_fast(fake: tuple[Fake, str]) -> None:
+ state, base = fake
+ state.script["/x"] = [(404, {})]
+ with pytest.raises(PermanentError):
+ gh_client(base).get_json("x")
+ assert len([c for c in state.calls if c[1] == "/x"]) == 1
+
+
+def test_get_exhausts_after_max_attempts(fake: tuple[Fake, str]) -> None:
+ state, base = fake
+ state.script["/x"] = [(500, {})] * 10
+ with pytest.raises(TransientError):
+ gh_client(base).get_json("x")
+ assert len([c for c in state.calls if c[1] == "/x"]) == FAST.max_attempts
+
+
+def test_post_retries_429_but_not_500(fake: tuple[Fake, str]) -> None:
+ state, base = fake
+ state.script["/w"] = [(429, {}), (201, {"id": 1})]
+ assert gh_client(base).request("POST", "w", json_body={}).json() == {"id": 1}
+ state.script["/w"] = [(500, {})]
+ with pytest.raises(AmbiguousWriteError):
+ gh_client(base).request("POST", "w", json_body={})
+ assert len([c for c in state.calls if c == ("POST", "/w")]) == 3
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Comment POST assumes body key</b></div>
<div id="fix">
`Handler._handle` indexes `payload["body"]` for POSTs to `*/comments`. Today the only producer is `GitHub.create_comment` (api.py:287), which always sends `body`; a future client posting without it crashes the handler thread (connection reset) instead of yielding a clean 4xx. `payload.get("body")` is the defensive form.
</div>
</div>
<div id="suggestion">
<div id="issue"><b>Missing docstrings on new tests</b></div>
<div id="fix">
The new fixture `fake`, helpers `gh_client`/`devin_client`, and the five tests have no docstrings. BITO.md adaptive rules [12148]/[12490] require docstrings on every new test function, fixture, and helper (type annotations are present; docstrings are not). Add one-liners stating scenario and expected outcome, e.g. on `test_get_retries_429_and_500_then_succeeds`.
</div>
</div>
<small><i>Code Review Run #8b0af2</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
automation/tests/test_orchestrator.py:
##########
@@ -0,0 +1,243 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""Behavioural tests for the orchestrator against a fake GitHub/Devin API.
+
+Run with ``python -m pytest automation/tests``.
+"""
+
+from __future__ import annotations
+
+import json # noqa: TID251 -- standalone tool, not part of the superset package
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Inline noqa vs per-file-ignores</b></div>
<div id="fix">
The repo bans stdlib `json` via ruff `flake8-tidy-imports.banned-api` (msg: "Use superset.utils.json instead"), and standalone modules opt out through `per-file-ignores` entries in `pyproject.toml` (`scripts/*`, `setup.py`, `superset/config.py`, `superset/utils/json.py`, etc.) โ never via inline `noqa`. Add an `automation/tests/*` per-file-ignores entry for TID251 and drop the inline suppression to match the established pattern.
</div>
</div>
<small><i>Code Review Run #8b0af2</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
automation/scripts/status.sh:
##########
@@ -0,0 +1,57 @@
+#!/usr/bin/env bash
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+# Shows whether the automation pipeline is healthy: which nightly-scan issues
+# are open, which of them already have a fix PR (branch devin/nightly-fix-<N>-*),
+# and, when DEVIN_API_KEY is set, the state of the Devin sessions behind them.
+#
+# Requires: gh (authenticated via GH_TOKEN or `gh auth login`), jq, curl.
+#
+set -euo pipefail
+
+REPO="${REPO:-moliyadhaval/superset}"
+ORG_ID="${DEVIN_ORG_ID:-org-eef03b923043402190c037ffa8141840}"
+
+echo "== Open nightly-scan issues on $REPO"
+gh issue list -R "$REPO" --state open --label nightly-scan --limit 300 \
+ --json number,title,createdAt > /tmp/issues.json
+gh pr list -R "$REPO" --state all --limit 500 --json number,state,headRefName,url > /tmp/prs.json
+
+jq -r --slurpfile prs /tmp/prs.json '
+ ($prs[0] | map(select(.headRefName | test("^devin/nightly-fix-(issue-)?[0-9]+")))
+ | map({key: (.headRefName | capture("nightly-fix-(issue-)?(?<n>[0-9]+)").n), value: "#\(.number) (\(.state))"})
+ | from_entries) as $byIssue
+ | sort_by(-.number)[]
+ | "\(.number)\t\(.createdAt[:10])\t\($byIssue[(.number|tostring)] // "no PR yet")\t\(.title[:70])"' /tmp/issues.json \
+ | column -t -s $'\t'
+
+echo
+echo "== Counts"
+jq -r 'length as $n | "open issues: \($n)"' /tmp/issues.json
+jq -r 'map(select(.headRefName | startswith("devin/"))) | group_by(.state) | map("\(.[0].state): \(length)") | "automation PRs by state: " + join(", ")' /tmp/prs.json
+
+if [ -n "${DEVIN_API_KEY:-}" ]; then
+ echo
+ echo "== Devin auto-fix sessions (latest 100)"
+ curl -sf -H "Authorization: Bearer ${DEVIN_API_KEY}" \
+ "https://api.devin.ai/v3/organizations/${ORG_ID}/sessions?tags=auto-fix&limit=100" \
+ | jq -r '(.items // .sessions // [])[] | "\(.status // .status_enum)\t\(.title)\thttps://app.devin.ai/sessions/\(.session_id | ltrimstr("devin-"))"' \
+ | column -t -s $'\t'
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Optional section aborts script</b></div>
<div id="fix">
With `set -euo pipefail`, a failed `curl -sf` (network/HTTP error) makes the pipeline fail and `set -e` aborts the script, discarding the issue/PR status already printed. This optional DEVIN_API_KEY section shouldn't kill the required output. Add `|| true` to the pipeline.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
| column -t -s $'\t' || true
````
</div>
</details>
</div>
<small><i>Code Review Run #8b0af2</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
automation/tests/test_orchestrator.py:
##########
@@ -0,0 +1,243 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""Behavioural tests for the orchestrator against a fake GitHub/Devin API.
+
+Run with ``python -m pytest automation/tests``.
+"""
+
+from __future__ import annotations
+
+import json # noqa: TID251 -- standalone tool, not part of the superset package
+import threading
+from collections.abc import Iterator
+from datetime import timedelta
+from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
+from typing import Any
+
+import pytest
+
+from automation.orchestrator import dispatch as ops
+from automation.orchestrator.api import (
+ AmbiguousWriteError,
+ Devin,
+ GitHub,
+ PermanentError,
+ RetryPolicy,
+ TransientError,
+)
+
+FAST = RetryPolicy(
+ max_attempts=4, initial=0.01, max_sleep=0.05, budget_seconds=5, timeout=1
+)
+
+
+class Fake:
+ """Scriptable API: ``script[path]`` is a list of (status, body) replies."""
+
+ def __init__(self) -> None:
+ self.script: dict[str, list[tuple[int, Any]]] = {}
+ self.calls: list[tuple[str, str]] = []
+ self.comments: list[dict[str, Any]] = []
+ self.sessions: list[dict[str, Any]] = []
+
+ def reply(self, method: str, path: str) -> tuple[int, Any, dict[str, str]]:
+ self.calls.append((method, path))
+ if path in self.script and self.script[path]:
+ status, body = self.script[path].pop(0)
+ headers = {"Retry-After": "0"} if status == 429 else {}
+ return status, body, headers
+ if path.endswith("/comments") and method == "GET":
+ return 200, self.comments, {}
+ if path.endswith("/comments") and method == "POST":
+ return 500, {"message": "boom"}, {} # landed, but reply lost
+ if path.startswith("/sessions") and method == "GET":
+ return 200, {"items": self.sessions, "has_next_page": False}, {}
+ if path == "/sessions" and method == "POST":
+ return 500, {"message": "boom"}, {}
+ return 200, [], {}
+
+
+@pytest.fixture
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing fixture docstring</b></div>
<div id="fix">
BITO adaptive rule 12148 requires every new test function and fixture to carry a docstring stating purpose and expected behavior. The `fake` fixture (line 72-73) spins up a `ThreadingHTTPServer` and yields a `(Fake, base_url)` tuple, none of which is self-evident from the name. Add a one-line docstring documenting what it yields and its server lifecycle.
</div>
</div>
<small><i>Code Review Run #8b0af2</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] Pr 205 [superset]
Posted by "hainenber (via GitHub)" <gi...@apache.org>.
hainenber commented on PR #44439:
URL: https://github.com/apache/superset/pull/44439#issuecomment-5741698556
Based on `README.md` changes in this PR, I assume the PR is meant to be merged to the author's forked mainline branch instead of upstream. Closing off.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] Pr 205 [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #44439:
URL: https://github.com/apache/superset/pull/44439#discussion_r4052312915
##########
automation/Dockerfile:
##########
@@ -0,0 +1,69 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+# Toolbox image for reproducing and validating the Devin automations that run
+# against this fork (see automation/README.md). It is NOT the Superset app image
+# (that is the top-level Dockerfile); it only bundles the scanners the nightly
+# scan uses plus the GitHub CLI, so anyone can run the same checks locally.
+#
+# docker build -t superset-automation -f automation/Dockerfile .
+# docker run --rm -it --user "$(id -u):$(id -g)" -v "$PWD":/workspace -e GH_TOKEN superset-automation
+#
+# Run with --user so the reports written to automation/out/ are owned by you,
+# not root. HOME is pre-created world-writable so gh/npm/pip-audit caches work
+# for any uid.
+#
+ARG PY_VER=3.11-slim-bookworm
+FROM python:${PY_VER}
+
+ARG NODE_MAJOR=24
+ARG RUFF_VERSION=0.9.7
+
+ENV PIP_DISABLE_PIP_VERSION_CHECK=1 \
+ PIP_NO_CACHE_DIR=1 \
+ PYTHONUNBUFFERED=1 \
+ HOME=/home/toolbox \
+ WORKSPACE=/workspace
+
+RUN apt-get update \
+ && apt-get install -y --no-install-recommends \
+ bsdextrautils ca-certificates curl git gnupg jq \
+ && mkdir -p /etc/apt/keyrings \
+ && curl -fsSL https://deb.nodesource.com/gpgkey/nodesource-repo.gpg.key \
+ | gpg --dearmor -o /etc/apt/keyrings/nodesource.gpg \
+ && echo "deb [signed-by=/etc/apt/keyrings/nodesource.gpg] https://deb.nodesource.com/node_${NODE_MAJOR}.x nodistro main" \
+ > /etc/apt/sources.list.d/nodesource.list \
+ && curl -fsSL https://cli.github.com/packages/githubcli-archive-keyring.gpg \
+ -o /etc/apt/keyrings/githubcli-archive-keyring.gpg \
+ && echo "deb [arch=$(dpkg --print-architecture) signed-by=/etc/apt/keyrings/githubcli-archive-keyring.gpg] https://cli.github.com/packages stable main" \
+ > /etc/apt/sources.list.d/github-cli.list \
+ && apt-get update \
+ && apt-get install -y --no-install-recommends nodejs gh \
+ && rm -rf /var/lib/apt/lists/*
+
+# Same scanners the nightly-scan automation installs. ruff is pinned to the
+# version in requirements/development.txt so results match CI.
+COPY automation/requirements.txt /tmp/automation-requirements.txt
+RUN pip install pip-audit bandit radon "ruff==${RUFF_VERSION}" \
+ -r /tmp/automation-requirements.txt
Review Comment:
**Suggestion:** Installing unpinned `pip-audit`, `bandit`, and `radon` makes future image builds use changing CLI behavior, so the toolbox cannot reliably reproduce the documented scan.
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Sometimes` ยท ๐ท๏ธ `Code quality`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=4883f3bc96c846c497c8a286513b7d0c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=4883f3bc96c846c497c8a286513b7d0c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** automation/Dockerfile
**Line:** 61:62
**Comment:**
*Code Quality: Installing unpinned `pip-audit`, `bandit`, and `radon` makes future image builds use changing CLI behavior, so the toolbox cannot reliably reproduce the documented scan.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44439&comment_hash=6fc94370dec86f3d1b8cac84cc7f8097c4bd9254ac9a3ebbf31c4a9027c35a83&reaction=like'>๐</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44439&comment_hash=6fc94370dec86f3d1b8cac84cc7f8097c4bd9254ac9a3ebbf31c4a9027c35a83&reaction=dislike'>๐</a>
##########
automation/scripts/scan.sh:
##########
@@ -0,0 +1,109 @@
+#!/usr/bin/env bash
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+# Runs the same read-only checks the nightly-scan automation runs, so a
+# finding in a `nightly-scan` issue can be reproduced locally. Nothing here
+# writes to GitHub or changes the repo.
+#
+# scan.sh [python-advisories|npm-advisories|outdated-deps|ci-supply-chain|frontend-lint|code-hardening|all]
+#
+set -uo pipefail
+
+ROOT="${WORKSPACE:-$(git rev-parse --show-toplevel 2>/dev/null || pwd)}"
+OUT="${SCAN_OUT:-$ROOT/automation/out}"
+mkdir -p "$OUT"
+cd "$ROOT"
+
+section() { printf '\n==== %s ====\n' "$1"; }
+
+python_advisories() {
+ section "python-advisories: pip-audit against pinned requirements"
+ for f in requirements/base.txt requirements/development.txt; do
+ echo "--- $f"
+ # --disable-pip audits the exact pins without building any wheel locally
+ pip-audit --disable-pip --no-deps -r "$f" -f json -o "$OUT/pip-audit-$(basename "$f" .txt).json" \
+ || case $? in
+ 1) ;; # exit 1 means vulnerabilities were found; the summary line above says how many
+ *) echo "pip-audit could not audit $f (the automation falls back to the OSV batch API)" ;;
+ esac
+ done
+}
+
+npm_advisories() {
+ section "npm-advisories: npm audit per lockfile workspace"
+ for ws in superset-frontend superset-websocket superset-embedded-sdk; do
+ [ -f "$ws/package-lock.json" ] || continue
+ echo "--- $ws"
+ (cd "$ws" && npm audit --audit-level=moderate --package-lock-only --json \
+ > "$OUT/npm-audit-$ws.json"; true)
Review Comment:
**Suggestion:** If `npm audit` fails because npm or the network is unavailable, `true` hides the failure and the script silently reports no findings from that workspace.
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Sometimes` ยท ๐ท๏ธ `Error handling`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=5eb0dde24efc49879e3accb7974fb23f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=5eb0dde24efc49879e3accb7974fb23f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** automation/scripts/scan.sh
**Line:** 51:52
**Comment:**
*Error Handling: If `npm audit` fails because npm or the network is unavailable, `true` hides the failure and the script silently reports no findings from that workspace.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44439&comment_hash=afebf2b14473c5e38cac4aea1b42118a8285a63edc2462c0a171831cf4193816&reaction=like'>๐</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44439&comment_hash=afebf2b14473c5e38cac4aea1b42118a8285a63edc2462c0a171831cf4193816&reaction=dislike'>๐</a>
##########
automation/scripts/scan.sh:
##########
@@ -0,0 +1,109 @@
+#!/usr/bin/env bash
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+# Runs the same read-only checks the nightly-scan automation runs, so a
+# finding in a `nightly-scan` issue can be reproduced locally. Nothing here
+# writes to GitHub or changes the repo.
+#
+# scan.sh [python-advisories|npm-advisories|outdated-deps|ci-supply-chain|frontend-lint|code-hardening|all]
+#
+set -uo pipefail
+
+ROOT="${WORKSPACE:-$(git rev-parse --show-toplevel 2>/dev/null || pwd)}"
+OUT="${SCAN_OUT:-$ROOT/automation/out}"
+mkdir -p "$OUT"
+cd "$ROOT"
+
+section() { printf '\n==== %s ====\n' "$1"; }
+
+python_advisories() {
+ section "python-advisories: pip-audit against pinned requirements"
+ for f in requirements/base.txt requirements/development.txt; do
+ echo "--- $f"
+ # --disable-pip audits the exact pins without building any wheel locally
+ pip-audit --disable-pip --no-deps -r "$f" -f json -o "$OUT/pip-audit-$(basename "$f" .txt).json" \
+ || case $? in
+ 1) ;; # exit 1 means vulnerabilities were found; the summary line above says how many
+ *) echo "pip-audit could not audit $f (the automation falls back to the OSV batch API)" ;;
+ esac
Review Comment:
**Suggestion:** When `pip-audit` fails for infrastructure reasons, this only prints a nonexistent fallback message and produces no advisory report, so Python vulnerabilities can be silently missed.
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Sometimes` ยท ๐ท๏ธ `Incomplete implementation`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=56a6991c7fc74c61bcee13a64e08404d&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=56a6991c7fc74c61bcee13a64e08404d&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** automation/scripts/scan.sh
**Line:** 38:42
**Comment:**
*Incomplete Implementation: When `pip-audit` fails for infrastructure reasons, this only prints a nonexistent fallback message and produces no advisory report, so Python vulnerabilities can be silently missed.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44439&comment_hash=539d5916a66013119c2651203bcdfa3edd940539fcd7216aadc8d9eea362f050&reaction=like'>๐</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44439&comment_hash=539d5916a66013119c2651203bcdfa3edd940539fcd7216aadc8d9eea362f050&reaction=dislike'>๐</a>
##########
automation/scripts/scan.sh:
##########
@@ -0,0 +1,109 @@
+#!/usr/bin/env bash
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+# Runs the same read-only checks the nightly-scan automation runs, so a
+# finding in a `nightly-scan` issue can be reproduced locally. Nothing here
+# writes to GitHub or changes the repo.
+#
+# scan.sh [python-advisories|npm-advisories|outdated-deps|ci-supply-chain|frontend-lint|code-hardening|all]
+#
+set -uo pipefail
+
+ROOT="${WORKSPACE:-$(git rev-parse --show-toplevel 2>/dev/null || pwd)}"
+OUT="${SCAN_OUT:-$ROOT/automation/out}"
+mkdir -p "$OUT"
+cd "$ROOT"
+
+section() { printf '\n==== %s ====\n' "$1"; }
+
+python_advisories() {
+ section "python-advisories: pip-audit against pinned requirements"
+ for f in requirements/base.txt requirements/development.txt; do
+ echo "--- $f"
+ # --disable-pip audits the exact pins without building any wheel locally
+ pip-audit --disable-pip --no-deps -r "$f" -f json -o "$OUT/pip-audit-$(basename "$f" .txt).json" \
+ || case $? in
+ 1) ;; # exit 1 means vulnerabilities were found; the summary line above says how many
+ *) echo "pip-audit could not audit $f (the automation falls back to the OSV batch API)" ;;
+ esac
+ done
+}
+
+npm_advisories() {
+ section "npm-advisories: npm audit per lockfile workspace"
+ for ws in superset-frontend superset-websocket superset-embedded-sdk; do
+ [ -f "$ws/package-lock.json" ] || continue
+ echo "--- $ws"
+ (cd "$ws" && npm audit --audit-level=moderate --package-lock-only --json \
+ > "$OUT/npm-audit-$ws.json"; true)
+ jq -r '.metadata.vulnerabilities // {} | to_entries[] | "\(.key): \(.value)"' \
+ "$OUT/npm-audit-$ws.json" 2>/dev/null || true
+ done
+}
+
+outdated_deps() {
+ section "outdated-deps: npm outdated in superset-frontend"
+ (cd superset-frontend && npm outdated --json > "$OUT/npm-outdated.json"; true)
+ jq -r 'to_entries[] | "\(.key): \(.value.current) -> \(.value.latest)"' \
+ "$OUT/npm-outdated.json" 2>/dev/null | head -50
+ echo "(Python pins are compared against PyPI by the automation; see requirements/base.in for deliberate caps.)"
+}
+
+ci_supply_chain() {
+ section "ci-supply-chain: risky patterns in .github/workflows"
+ grep -rn "pull_request_target" .github/workflows || echo "no pull_request_target"
+ echo "--- third-party actions not pinned to a commit SHA"
+ grep -rhoE "uses: [^@]+@[^ ]+" .github/workflows \
+ | grep -vE "@[0-9a-f]{40}$" | grep -v "uses: ./" | sort | uniq -c | sort -rn | head -30
+ echo "--- unpinned installs"
+ grep -rnE "(pip|npm) install " .github/workflows | grep -vE "==|@[0-9]" | head -20
+}
+
+frontend_lint() {
+ section "frontend-lint: oxlint warnings in production code (needs node_modules)"
+ if [ ! -d superset-frontend/node_modules ]; then
+ echo "superset-frontend/node_modules missing; run 'npm ci' there first (slow, ~5 min)"
+ return
+ fi
+ (cd superset-frontend && npx oxlint --config oxlint.json --format json > "$OUT/oxlint.json"; true)
Review Comment:
**Suggestion:** If `npx oxlint` cannot run or fails to load its configuration, `true` hides the error and the scan continues as though production linting found nothing.
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Sometimes` ยท ๐ท๏ธ `Error handling`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=14b9a1da8bfb438589bee009524d3237&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=14b9a1da8bfb438589bee009524d3237&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** automation/scripts/scan.sh
**Line:** 82:82
**Comment:**
*Error Handling: If `npx oxlint` cannot run or fails to load its configuration, `true` hides the error and the scan continues as though production linting found nothing.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44439&comment_hash=1252fbf3717b728129800871b0d7508ffdbd6f08d4920882551a10b9c34bfd2c&reaction=like'>๐</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44439&comment_hash=1252fbf3717b728129800871b0d7508ffdbd6f08d4920882551a10b9c34bfd2c&reaction=dislike'>๐</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] Pr 205 [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #44439:
URL: https://github.com/apache/superset/pull/44439#discussion_r4052314354
##########
automation/orchestrator/__main__.py:
##########
@@ -0,0 +1,128 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""Command line entry point: ``python -m automation.orchestrator <command>``."""
+
+from __future__ import annotations
+
+import argparse
+import logging
+import os
+import sys
+from datetime import datetime, timedelta, timezone
+
+from . import dispatch as ops
+from .api import Devin, GitHub
+
+REPO = os.environ.get("REPO", "moliyadhaval/superset")
+ORG_ID = os.environ.get("DEVIN_ORG_ID", "org-eef03b923043402190c037ffa8141840")
+DASHBOARD_ISSUE = int(os.environ.get("DASHBOARD_ISSUE", "18"))
+
+
+def _require(*names: str) -> None:
+ missing = [n for n in names if not os.environ.get(n)]
+ if missing:
+ sys.exit(f"missing environment variable(s): {', '.join(missing)}")
+
+
+def main(argv: list[str] | None = None) -> int:
+ parser = argparse.ArgumentParser(prog="automation.orchestrator")
+ parser.add_argument("-v", "--verbose", action="store_true")
+ sub = parser.add_subparsers(dest="cmd", required=True)
+
+ p = sub.add_parser("dispatch", help="dispatch one fix session per eligible issue")
+ p.add_argument(
+ "--prompt-file", required=True, help="file holding the canonical fix prompt"
+ )
+ p.add_argument("--cap", type=int, default=30)
+ p.add_argument("--dry-run", action="store_true")
+
+ p = sub.add_parser(
+ "watch-session", help="poll a session; fail unless it produced a PR"
+ )
+ p.add_argument("session_id")
+ p.add_argument("--issue", type=int, required=True)
+ p.add_argument("--timeout-minutes", type=int, default=90)
+ p.add_argument("--poll-seconds", type=float, default=60)
+ p.add_argument(
+ "--keep-branch",
+ action="store_true",
+ help="do not delete the orphan branch on failure",
+ )
+
+ p = sub.add_parser(
+ "stale-branches", help="list (or delete) fix branches without an open/merged PR"
+ )
+ p.add_argument("--days", type=int, default=int(os.environ.get("STALE_DAYS", "3")))
+ p.add_argument("--delete", action="store_true")
+
+ args = parser.parse_args(argv)
+ logging.basicConfig(
+ level=logging.DEBUG if args.verbose else logging.INFO,
+ format="%(asctime)s %(levelname)s %(name)s: %(message)s",
+ stream=sys.stderr,
+ )
+ gh = GitHub(REPO)
+ devin = Devin(ORG_ID)
+ today = datetime.now(timezone.utc).date().isoformat()
+
+ if args.cmd == "dispatch":
+ _require("GH_TOKEN", *([] if args.dry_run else ["DEVIN_API_KEY"]))
+ with open(args.prompt_file, encoding="utf-8") as fh:
+ prompt = fh.read()
+ outcomes = ops.dispatch(
+ gh, devin, fix_prompt=prompt, cap=args.cap, dry_run=args.dry_run
+ )
+ table = ops.dashboard_table(outcomes)
+ print(table)
+ if not args.dry_run:
+ ops.ensure_comment(
+ gh,
+ DASHBOARD_ISSUE,
+ f"dispatch:{today}",
+ f"### Dispatch run {today}\n\n{table}",
+ )
+ return 1 if any(o.status == "failed" for o in outcomes) else 0
+
+ if args.cmd == "watch-session":
+ _require("GH_TOKEN", "DEVIN_API_KEY")
+ ok, detail = False, "watch aborted"
+ try:
+ ok, detail = ops.watch_session(
+ devin,
+ args.session_id,
+ timeout=timedelta(minutes=args.timeout_minutes),
+ poll=args.poll_seconds,
+ )
+ finally:
+ verdict = "โ
PR opened" if ok else "โ Failed"
+ line = f"[Issue #{args.issue}] {verdict} - {detail}"
+ logging.getLogger("automation").log(
+ logging.INFO if ok else logging.ERROR, line
+ )
+ ops.ensure_comment(gh, DASHBOARD_ISSUE, f"session:{args.session_id}", line)
+ if not ok and not args.keep_branch:
+ ops.cleanup_branch(gh, args.issue)
Review Comment:
**Suggestion:** An exception from `ensure_comment` replaces the polling result and prevents `cleanup_branch`, leaving failed sessions' orphan branches behind.
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Sometimes` ยท ๐ท๏ธ `Missing cleanup`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=ac2edd86564f49c6901ae2f550f16d0a&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=ac2edd86564f49c6901ae2f550f16d0a&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** automation/orchestrator/__main__.py
**Line:** 108:116
**Comment:**
*Missing Cleanup: An exception from `ensure_comment` replaces the polling result and prevents `cleanup_branch`, leaving failed sessions' orphan branches behind.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44439&comment_hash=5846e7ed93adabd06f9e7e7592acea958bfa2b5922f210f2a0fb27ebe18b8792&reaction=like'>๐</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44439&comment_hash=5846e7ed93adabd06f9e7e7592acea958bfa2b5922f210f2a0fb27ebe18b8792&reaction=dislike'>๐</a>
##########
automation/orchestrator/api.py:
##########
@@ -0,0 +1,342 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""HTTP clients for GitHub and the Devin API with a shared retry policy.
+
+Retry contract
+--------------
+* ``429`` and rate-limited ``403`` (primary/secondary/abuse) are retried for
+ every method: the server rejected the request, nothing happened.
+* ``5xx`` and transport errors (timeouts, resets) are retried for idempotent
+ methods (GET/HEAD/PUT/DELETE). For POST/PATCH they raise
+ :class:`AmbiguousWriteError` so the caller probes for the resource before
+ writing again -- this is what keeps retries idempotent.
+* Other ``4xx`` raise :class:`PermanentError` immediately.
+* Waits use exponential backoff with full jitter (tenacity), capped per
+ attempt and by a total budget, and honour ``Retry-After`` /
+ ``X-RateLimit-Reset`` when present.
+
+Secrets are only ever placed in request headers; they are never logged.
+"""
+
+from __future__ import annotations
+
+import email.utils
+import logging
+import os
+import time
+from collections.abc import Callable, Iterator
+from dataclasses import dataclass, field
+from typing import Any
+
+import requests
+from tenacity import (
+ retry_if_exception_type,
+ RetryCallState,
+ Retrying,
+ stop_after_attempt,
+ stop_after_delay,
+ wait_exponential_jitter,
+)
+
+log = logging.getLogger("automation.api")
+
+IDEMPOTENT_METHODS = frozenset({"GET", "PUT", "DELETE"})
+RATE_LIMIT_MARKERS = ("rate limit", "abuse detection", "too many requests")
+
+
+class ApiError(Exception):
+ """Base class for API failures."""
+
+ def __init__(self, message: str, status: int | None = None) -> None:
+ super().__init__(message)
+ self.status = status
+
+
+class TransientError(ApiError):
+ """Safe to retry: the request was rejected or the read failed."""
+
+ def __init__(
+ self, message: str, status: int | None = None, retry_after: float | None = None
+ ) -> None:
+ super().__init__(message, status)
+ self.retry_after = retry_after
+
+
+class AmbiguousWriteError(ApiError):
+ """A write may or may not have landed; probe before repeating it."""
+
+
+class PermanentError(ApiError):
+ """Client error that will not succeed on retry (401/404/422...)."""
+
+
+@dataclass(frozen=True)
+class RetryPolicy:
+ max_attempts: int = int(os.environ.get("RETRY_MAX_ATTEMPTS", "6"))
+ initial: float = float(os.environ.get("RETRY_BASE_SECONDS", "2"))
+ max_sleep: float = float(os.environ.get("RETRY_MAX_SLEEP", "120"))
+ budget_seconds: float = float(os.environ.get("RETRY_BUDGET_SECONDS", "900"))
+ timeout: float = float(os.environ.get("HTTP_TIMEOUT", "60"))
+
+
+def _wait(policy: RetryPolicy) -> Callable[[RetryCallState], float]:
+ """Exponential backoff with full jitter, overridden by a server hint."""
+ jitter = wait_exponential_jitter(initial=policy.initial, max=policy.max_sleep)
+
+ def wait(state: RetryCallState) -> float:
+ exc = state.outcome.exception() if state.outcome else None
+ if isinstance(exc, TransientError) and exc.retry_after is not None:
+ return min(max(exc.retry_after, 0.0), policy.max_sleep)
+ return float(jitter(state))
+
+ return wait
+
+
+def _retry_after_seconds(headers: Any) -> float | None:
+ """Parse Retry-After (seconds or HTTP-date) or X-RateLimit-Reset (epoch)."""
+ if raw := headers.get("Retry-After"):
+ try:
+ return float(raw)
+ except ValueError:
+ parsed = email.utils.parsedate_to_datetime(raw)
+ return max(parsed.timestamp() - time.time(), 0.0)
Review Comment:
**Suggestion:** A malformed nonnumeric `Retry-After` header makes date parsing raise an uncaught `ValueError`, aborting an otherwise retryable request.
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Rarely` ยท ๐ท๏ธ `Possible bug`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f720560ca9814df08338fbdc4763bcd3&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=f720560ca9814df08338fbdc4763bcd3&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** automation/orchestrator/api.py
**Line:** 109:114
**Comment:**
*Possible Bug: A malformed nonnumeric `Retry-After` header makes date parsing raise an uncaught `ValueError`, aborting an otherwise retryable request.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44439&comment_hash=7c7747d05624c6553271a93a1bd8bea9cbcd93c83b70ec0b0934468d441e4a7a&reaction=like'>๐</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44439&comment_hash=7c7747d05624c6553271a93a1bd8bea9cbcd93c83b70ec0b0934468d441e4a7a&reaction=dislike'>๐</a>
##########
automation/orchestrator/dispatch.py:
##########
@@ -0,0 +1,290 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""Dispatch one Devin fix session per open nightly-scan issue, safely.
+
+Every step is idempotent: a session is looked up by title before it is
+created, a dashboard comment is looked up by a hidden marker before it is
+posted, and a failure for one issue is recorded and the loop moves on.
+"""
+
+from __future__ import annotations
+
+import logging
+import re
+import time
+from collections.abc import Callable, Iterable
+from dataclasses import dataclass
+from datetime import datetime, timedelta, timezone
+from typing import Any
+
+from .api import AmbiguousWriteError, ApiError, Devin, GitHub, session_url
+
+log = logging.getLogger("automation.dispatch")
+
+BRANCH_PREFIX = "devin/nightly-fix-"
+BRANCH_ISSUE = re.compile(r"devin/nightly-fix-(?:issue-)?(\d+)")
+BODY_REF = re.compile(r"\b(?:Fixes|Refs|Closes)\s+#(\d+)", re.I)
+DEAD_STATUSES = frozenset({"error", "terminated", "cancelled", "canceled"})
+FINISHED_STATUSES = frozenset({"finished", "completed", "blocked", "suspended"})
+
+
+@dataclass
+class Outcome:
+ issue: int
+ status: str # dispatched | skipped | failed
+ detail: str = ""
+
+
+def marker(key: str) -> str:
+ return f"<!-- devin:{key} -->"
+
+
+def attempted_issues(
+ prs: Iterable[dict[str, Any]], sessions: Iterable[dict[str, Any]]
+) -> set[int]:
+ """Issue numbers that already have a PR, a fix branch, or a live session."""
+ seen: set[int] = set()
+ for pr in prs:
+ seen.update(int(n) for n in BODY_REF.findall(pr.get("body") or ""))
+ if m := BRANCH_ISSUE.search(pr["head"]["ref"]):
+ seen.add(int(m.group(1)))
+ for s in sessions:
+ if str(s.get("status") or s.get("status_enum") or "").lower() in DEAD_STATUSES:
+ continue
+ if m := re.search(r"#(\d+)", s.get("title") or ""):
+ seen.add(int(m.group(1)))
+ return seen
+
+
+def ensure_session(devin: Devin, payload: dict[str, Any], tag: str) -> dict[str, Any]:
+ """Create a session unless one with the same title is already alive.
+
+ An ambiguous failure (5xx/timeout on the POST) is resolved by probing
+ again, so a request that landed is never duplicated.
+ """
+ title = payload["title"]
+ for attempt in (1, 2):
+ for s in devin.list_sessions(tag):
+ if (
+ s.get("title") == title
+ and str(s.get("status") or s.get("status_enum") or "").lower()
+ not in DEAD_STATUSES
+ ):
+ log.info("%s already has session %s", title, session_url(s))
+ return s
+ try:
+ return devin.create_session(payload)
Review Comment:
**Suggestion:** Concurrent dispatchers can both observe no matching session and create duplicate sessions because the lookup and creation are separate requests.
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Rarely` ยท ๐ท๏ธ `Race condition`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=31699b41175040c6bdd05473dc3a3f19&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=31699b41175040c6bdd05473dc3a3f19&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** automation/orchestrator/dispatch.py
**Line:** 79:88
**Comment:**
*Race Condition: Concurrent dispatchers can both observe no matching session and create duplicate sessions because the lookup and creation are separate requests.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44439&comment_hash=9b7625a45277bd852385448382c97745088822003e6d61a2cb3db422bcdd06bb&reaction=like'>๐</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44439&comment_hash=9b7625a45277bd852385448382c97745088822003e6d61a2cb3db422bcdd06bb&reaction=dislike'>๐</a>
##########
automation/orchestrator/dispatch.py:
##########
@@ -0,0 +1,290 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""Dispatch one Devin fix session per open nightly-scan issue, safely.
+
+Every step is idempotent: a session is looked up by title before it is
+created, a dashboard comment is looked up by a hidden marker before it is
+posted, and a failure for one issue is recorded and the loop moves on.
+"""
+
+from __future__ import annotations
+
+import logging
+import re
+import time
+from collections.abc import Callable, Iterable
+from dataclasses import dataclass
+from datetime import datetime, timedelta, timezone
+from typing import Any
+
+from .api import AmbiguousWriteError, ApiError, Devin, GitHub, session_url
+
+log = logging.getLogger("automation.dispatch")
+
+BRANCH_PREFIX = "devin/nightly-fix-"
+BRANCH_ISSUE = re.compile(r"devin/nightly-fix-(?:issue-)?(\d+)")
+BODY_REF = re.compile(r"\b(?:Fixes|Refs|Closes)\s+#(\d+)", re.I)
+DEAD_STATUSES = frozenset({"error", "terminated", "cancelled", "canceled"})
+FINISHED_STATUSES = frozenset({"finished", "completed", "blocked", "suspended"})
+
+
+@dataclass
+class Outcome:
+ issue: int
+ status: str # dispatched | skipped | failed
+ detail: str = ""
+
+
+def marker(key: str) -> str:
+ return f"<!-- devin:{key} -->"
+
+
+def attempted_issues(
+ prs: Iterable[dict[str, Any]], sessions: Iterable[dict[str, Any]]
+) -> set[int]:
+ """Issue numbers that already have a PR, a fix branch, or a live session."""
+ seen: set[int] = set()
+ for pr in prs:
+ seen.update(int(n) for n in BODY_REF.findall(pr.get("body") or ""))
+ if m := BRANCH_ISSUE.search(pr["head"]["ref"]):
+ seen.add(int(m.group(1)))
+ for s in sessions:
+ if str(s.get("status") or s.get("status_enum") or "").lower() in DEAD_STATUSES:
+ continue
+ if m := re.search(r"#(\d+)", s.get("title") or ""):
+ seen.add(int(m.group(1)))
+ return seen
+
+
+def ensure_session(devin: Devin, payload: dict[str, Any], tag: str) -> dict[str, Any]:
+ """Create a session unless one with the same title is already alive.
+
+ An ambiguous failure (5xx/timeout on the POST) is resolved by probing
+ again, so a request that landed is never duplicated.
+ """
+ title = payload["title"]
+ for attempt in (1, 2):
+ for s in devin.list_sessions(tag):
+ if (
+ s.get("title") == title
+ and str(s.get("status") or s.get("status_enum") or "").lower()
+ not in DEAD_STATUSES
+ ):
+ log.info("%s already has session %s", title, session_url(s))
+ return s
+ try:
+ return devin.create_session(payload)
+ except AmbiguousWriteError as exc:
+ log.warning("%s: %s. Re-probing (attempt %d/2)", title, exc, attempt)
+ time.sleep(devin.policy.initial)
+ raise ApiError(f"{title}: session creation ambiguous after re-probe")
+
+
+def ensure_comment(gh: GitHub, issue: int, key: str, body: str) -> dict[str, Any]:
+ """Post ``body`` on ``issue`` once per ``key``; update it if it exists."""
+ text = f"{body}\n\n{marker(key)}"
+ for c in gh.comments(issue):
+ if marker(key) in (c.get("body") or ""):
+ return gh.update_comment(int(c["id"]), text) if c["body"] != text else c
+ try:
+ return gh.create_comment(issue, text)
+ except AmbiguousWriteError:
+ for c in gh.comments(issue):
+ if marker(key) in (c.get("body") or ""):
+ return c
+ raise
+
+
+def watch_session(
+ devin: Devin,
+ session_id: str,
+ *,
+ timeout: timedelta,
+ poll: float = 60.0,
+ sleep: Callable[[float], None] = time.sleep,
+) -> tuple[bool, str]:
+ """Poll a session until it finishes, with an orchestrator-side circuit breaker.
+
+ Success requires *both* a finished status and a pull request on the
+ session; a "completed" session with no pull request is a failure. The v3
+ API exposes ``pull_requests: [{pr_url, pr_state}]`` (older payloads used a
+ single ``pull_request`` object); both shapes are accepted.
+ """
+ deadline = time.monotonic() + timeout.total_seconds()
+ attempt = 0
+ while True:
+ attempt += 1
+ s = devin.get_session(session_id)
+ status = str(s.get("status") or s.get("status_enum") or "").lower()
+ pr = session_pr(s)
+ log.info(
+ "[Session %s] poll %d: status=%s pr=%s",
+ session_id,
+ attempt,
+ status,
+ bool(pr),
+ )
+ if pr:
+ return True, pr
+ if status in DEAD_STATUSES:
+ return False, f"session {status}"
+ if status in FINISHED_STATUSES:
+ return False, f"session {status} but pull_request is null"
+ if time.monotonic() >= deadline:
+ return False, f"timeout reached after {timeout} (last status: {status})"
+ sleep(poll)
+
+
+def session_pr(session: dict[str, Any]) -> str | None:
+ """URL of the session's pull request, or None when it has not opened one."""
+ prs = list(session.get("pull_requests") or [])
+ if session.get("pull_request"):
+ prs.append(session["pull_request"])
+ for pr in prs:
+ if url := pr.get("pr_url") or pr.get("url"):
+ return str(url)
+ return None
+
+
+def branch_for_issue(gh: GitHub, issue: int) -> str | None:
+ for b in gh.paginate("branches"):
+ if (m := BRANCH_ISSUE.match(b["name"])) and int(m.group(1)) == issue:
+ return str(b["name"])
+ return None
+
+
+def cleanup_branch(gh: GitHub, issue: int, dry_run: bool = False) -> str | None:
+ """Delete the fix branch for ``issue`` unless a PR (any state) uses it."""
+ branch = branch_for_issue(gh, issue)
+ if not branch:
+ return None
+ if any(pr["head"]["ref"] == branch for pr in gh.pulls("all")):
+ log.info("[Issue #%d] keeping %s: referenced by a PR", issue, branch)
+ return None
+ if not dry_run:
+ gh.delete_branch(branch)
Review Comment:
**Suggestion:** The branch usage check and deletion are separate requests, so a newly opened pull request can be missed and its branch deleted.
**Assessment:** ๐ด `Critical` ยท ๐ `Occurrence: Rarely` ยท ๐ท๏ธ `Race condition`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=53c3552ae800417ba3cee589bf0897aa&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=53c3552ae800417ba3cee589bf0897aa&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** automation/orchestrator/dispatch.py
**Line:** 173:177
**Comment:**
*Race Condition: The branch usage check and deletion are separate requests, so a newly opened pull request can be missed and its branch deleted.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44439&comment_hash=12de1a73052bbbeb18ca9bae79da89322dd15d6f95e2053d00196c9929f0f434&reaction=like'>๐</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44439&comment_hash=12de1a73052bbbeb18ca9bae79da89322dd15d6f95e2053d00196c9929f0f434&reaction=dislike'>๐</a>
##########
automation/orchestrator/dispatch.py:
##########
@@ -0,0 +1,290 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""Dispatch one Devin fix session per open nightly-scan issue, safely.
+
+Every step is idempotent: a session is looked up by title before it is
+created, a dashboard comment is looked up by a hidden marker before it is
+posted, and a failure for one issue is recorded and the loop moves on.
+"""
+
+from __future__ import annotations
+
+import logging
+import re
+import time
+from collections.abc import Callable, Iterable
+from dataclasses import dataclass
+from datetime import datetime, timedelta, timezone
+from typing import Any
+
+from .api import AmbiguousWriteError, ApiError, Devin, GitHub, session_url
+
+log = logging.getLogger("automation.dispatch")
+
+BRANCH_PREFIX = "devin/nightly-fix-"
+BRANCH_ISSUE = re.compile(r"devin/nightly-fix-(?:issue-)?(\d+)")
+BODY_REF = re.compile(r"\b(?:Fixes|Refs|Closes)\s+#(\d+)", re.I)
+DEAD_STATUSES = frozenset({"error", "terminated", "cancelled", "canceled"})
+FINISHED_STATUSES = frozenset({"finished", "completed", "blocked", "suspended"})
+
+
+@dataclass
+class Outcome:
+ issue: int
+ status: str # dispatched | skipped | failed
+ detail: str = ""
+
+
+def marker(key: str) -> str:
+ return f"<!-- devin:{key} -->"
+
+
+def attempted_issues(
+ prs: Iterable[dict[str, Any]], sessions: Iterable[dict[str, Any]]
+) -> set[int]:
+ """Issue numbers that already have a PR, a fix branch, or a live session."""
+ seen: set[int] = set()
+ for pr in prs:
+ seen.update(int(n) for n in BODY_REF.findall(pr.get("body") or ""))
+ if m := BRANCH_ISSUE.search(pr["head"]["ref"]):
+ seen.add(int(m.group(1)))
+ for s in sessions:
+ if str(s.get("status") or s.get("status_enum") or "").lower() in DEAD_STATUSES:
+ continue
+ if m := re.search(r"#(\d+)", s.get("title") or ""):
+ seen.add(int(m.group(1)))
+ return seen
+
+
+def ensure_session(devin: Devin, payload: dict[str, Any], tag: str) -> dict[str, Any]:
+ """Create a session unless one with the same title is already alive.
+
+ An ambiguous failure (5xx/timeout on the POST) is resolved by probing
+ again, so a request that landed is never duplicated.
+ """
+ title = payload["title"]
+ for attempt in (1, 2):
+ for s in devin.list_sessions(tag):
+ if (
+ s.get("title") == title
+ and str(s.get("status") or s.get("status_enum") or "").lower()
+ not in DEAD_STATUSES
+ ):
+ log.info("%s already has session %s", title, session_url(s))
+ return s
+ try:
+ return devin.create_session(payload)
+ except AmbiguousWriteError as exc:
+ log.warning("%s: %s. Re-probing (attempt %d/2)", title, exc, attempt)
+ time.sleep(devin.policy.initial)
+ raise ApiError(f"{title}: session creation ambiguous after re-probe")
+
+
+def ensure_comment(gh: GitHub, issue: int, key: str, body: str) -> dict[str, Any]:
+ """Post ``body`` on ``issue`` once per ``key``; update it if it exists."""
+ text = f"{body}\n\n{marker(key)}"
+ for c in gh.comments(issue):
+ if marker(key) in (c.get("body") or ""):
+ return gh.update_comment(int(c["id"]), text) if c["body"] != text else c
+ try:
+ return gh.create_comment(issue, text)
+ except AmbiguousWriteError:
+ for c in gh.comments(issue):
+ if marker(key) in (c.get("body") or ""):
+ return c
+ raise
+
+
+def watch_session(
+ devin: Devin,
+ session_id: str,
+ *,
+ timeout: timedelta,
+ poll: float = 60.0,
+ sleep: Callable[[float], None] = time.sleep,
+) -> tuple[bool, str]:
+ """Poll a session until it finishes, with an orchestrator-side circuit breaker.
+
+ Success requires *both* a finished status and a pull request on the
+ session; a "completed" session with no pull request is a failure. The v3
+ API exposes ``pull_requests: [{pr_url, pr_state}]`` (older payloads used a
+ single ``pull_request`` object); both shapes are accepted.
+ """
+ deadline = time.monotonic() + timeout.total_seconds()
+ attempt = 0
+ while True:
+ attempt += 1
+ s = devin.get_session(session_id)
+ status = str(s.get("status") or s.get("status_enum") or "").lower()
+ pr = session_pr(s)
+ log.info(
+ "[Session %s] poll %d: status=%s pr=%s",
+ session_id,
+ attempt,
+ status,
+ bool(pr),
+ )
+ if pr:
+ return True, pr
+ if status in DEAD_STATUSES:
+ return False, f"session {status}"
+ if status in FINISHED_STATUSES:
+ return False, f"session {status} but pull_request is null"
+ if time.monotonic() >= deadline:
+ return False, f"timeout reached after {timeout} (last status: {status})"
+ sleep(poll)
+
+
+def session_pr(session: dict[str, Any]) -> str | None:
+ """URL of the session's pull request, or None when it has not opened one."""
+ prs = list(session.get("pull_requests") or [])
+ if session.get("pull_request"):
+ prs.append(session["pull_request"])
+ for pr in prs:
+ if url := pr.get("pr_url") or pr.get("url"):
+ return str(url)
+ return None
+
+
+def branch_for_issue(gh: GitHub, issue: int) -> str | None:
+ for b in gh.paginate("branches"):
+ if (m := BRANCH_ISSUE.match(b["name"])) and int(m.group(1)) == issue:
+ return str(b["name"])
+ return None
+
+
+def cleanup_branch(gh: GitHub, issue: int, dry_run: bool = False) -> str | None:
+ """Delete the fix branch for ``issue`` unless a PR (any state) uses it."""
+ branch = branch_for_issue(gh, issue)
+ if not branch:
+ return None
+ if any(pr["head"]["ref"] == branch for pr in gh.pulls("all")):
+ log.info("[Issue #%d] keeping %s: referenced by a PR", issue, branch)
+ return None
+ if not dry_run:
+ gh.delete_branch(branch)
+ log.info("[Issue #%d] deleted orphaned branch %s", issue, branch)
+ return branch
+
+
+def stale_branches(
+ gh: GitHub, older_than: timedelta, *, delete: bool = False
+) -> list[tuple[str, str]]:
+ """Fix branches with no open/merged PR and a last commit older than the cutoff."""
+ prs: dict[str, str] = {}
+ for pr in gh.pulls("all"):
+ state = "MERGED" if pr.get("merged_at") else str(pr["state"]).upper()
+ prs[pr["head"]["ref"]] = (
+ state if prs.get(pr["head"]["ref"]) != "OPEN" else "OPEN"
+ )
Review Comment:
**Suggestion:** When a merged and later closed pull request share a branch, this assignment can overwrite `MERGED` with `CLOSED`, allowing stale cleanup to delete the branch.
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Rarely` ยท ๐ท๏ธ `Incorrect condition logic`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=6ab6f43bf20b468d88db959af3708934&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=6ab6f43bf20b468d88db959af3708934&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** automation/orchestrator/dispatch.py
**Line:** 187:191
**Comment:**
*Incorrect Condition Logic: When a merged and later closed pull request share a branch, this assignment can overwrite `MERGED` with `CLOSED`, allowing stale cleanup to delete the branch.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44439&comment_hash=209ea1991e5eb603a933b023be4169462c7b442ccd7f718b63c15002bc0527da&reaction=like'>๐</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44439&comment_hash=209ea1991e5eb603a933b023be4169462c7b442ccd7f718b63c15002bc0527da&reaction=dislike'>๐</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] Pr 205 [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44439:
URL: https://github.com/apache/superset/pull/44439#issuecomment-5739546860
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org