Skip to content

Remove redundant imports & sort them in native.py#671

Open
wyattscarpenter wants to merge 2 commits into
databricks:mainfrom
wyattscarpenter:patch-3
Open

Remove redundant imports & sort them in native.py#671
wyattscarpenter wants to merge 2 commits into
databricks:mainfrom
wyattscarpenter:patch-3

Conversation

@wyattscarpenter

Copy link
Copy Markdown
Contributor

What type of PR is this?

  • Refactor

Description

Remove redundant imports & sort them in native.py

How is this tested?

current CI

Related Tickets & Documents

n/a

@wyattscarpenter

wyattscarpenter commented Aug 8, 2025

Copy link
Copy Markdown
Contributor Author

I feel like the CI failures might not be related to my changes.

Signed-off-by: wyattscarpenter <wyattscarpenter@gmail.com>
Ali-Razmjoo pushed a commit to fork-the-planet/databricks___databricks-sql-python that referenced this pull request Jul 9, 2026
…atabricks#831) (databricks#845)

Fork pull_request runs never receive a GitHub OIDC token, so
.github/actions/setup-jfrog (which mints a JFrog PyPI token from that OIDC
token) dies with "ACTIONS_ID_TOKEN_REQUEST_TOKEN: unbound variable" and every
required check goes red on every fork PR — regardless of the diff (databricks#831,
reported against the trivial fork PR databricks#671). Public PyPI is unreachable from the
protected runners, so a fork run can obtain no package source at all.

Adopt the Databricks SOP (mirrors databricks/dbt-databricks): a trusted warmer
workflow that DOES get OIDC pre-builds the in-project .venv for every matrix
leg and saves it to the Actions cache; fork PRs restore that .venv and run the
tools directly from .venv/bin, fully offline, never touching JFrog. Everything
still flows through JFrog (the warmer populates the cache from it); the fork
just holds no credentials, so there is no pull_request_target / secret-exposure
risk.

- .github/workflows/warm-deps-cache.yml: new, sole writer of the forkvenv-*
  cache. Triggers on push:main (dep files), a daily schedule (beats GitHub's
  7-day cache eviction), and workflow_dispatch with an optional pr_number that
  warms a fork's changed lockfiles (fetches ONLY poetry.lock/pyproject.toml, no
  source). Warms the full py x depset x extras matrix, priming mypy stubs too.
- .github/actions/restore-deps: new composite action; restores the newest
  forkvenv-* entry for the leg via a restore-keys prefix and reports cache-hit.
- .github/workflows/code-quality-checks.yml: each job tries restore-deps first
  and runs offline from .venv/bin on a hit; on a miss (all same-repo PRs, or a
  fork whose lockfiles changed pre-warm) it falls back to the original
  setup-poetry JFrog flow, unchanged. Same-repo PR behavior is unaffected.

Scope: unit + lint + type checks (the fork-reachable required checks). e2e /
kernel-e2e / integration / DBR-LTS stay merge-queue-gated (they need live
warehouse secrets).

Closes databricks#831

Co-authored-by: Isaac

Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
@wyattscarpenter wyattscarpenter marked this pull request as draft July 9, 2026 03:17
@wyattscarpenter wyattscarpenter marked this pull request as ready for review July 9, 2026 03:18
vikrantpuppala added a commit that referenced this pull request Jul 9, 2026
The fork verification (#849) showed 3 checks still red on fork PRs — pre-existing
(also red on the original #671), separate from the cache fix, all rooted in
GitHub's fork restrictions:

- skip-integration-tests-pr / skip-kernel-e2e-pr post synthetic-success
  check-runs, needing checks:write. On fork PRs GitHub forces GITHUB_TOKEN to
  read-only regardless of the declared permission, so checks.create 403s
  ('Resource not accessible by integration'). Swallow the 403 ONLY for forks
  (github.event.pull_request.head.repo.fork) so the poster doesn't show a
  spurious failure; the REAL required checks are posted by the merge_group run
  (base context, full perms) when a maintainer queues the PR — the fork's code
  is genuinely integration-tested there. Non-fork/other errors still fail loudly.

- test-with-coverage runs the e2e suite against a live warehouse and needs the
  azure-prod secrets, which forks never get (Secret source: None) — so it could
  only fail on a fork. Skip it on fork PRs; the real coverage gate runs on the
  merge_group transient branch (secrets present), which is what protects main.

Same-repo PRs and merge_group are unaffected by all three changes.

Refs #831

Co-authored-by: Isaac

Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant