Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
fc348b4
first pass
joshuarli Aug 19, 2025
a70f01c
ignore some and fix some
joshuarli Aug 19, 2025
5d556eb
repin
joshuarli Aug 20, 2025
5f8b910
fix
joshuarli Aug 20, 2025
b16ecd7
add py as workspace
joshuarli Aug 20, 2025
73fb625
.
joshuarli Aug 20, 2025
8c99e1f
incrementally run pre-commit
joshuarli Aug 20, 2025
047f166
pc
joshuarli Aug 20, 2025
8d5db14
regenerate lockfile
joshuarli Aug 20, 2025
cfbc3ec
[skip ci] reduce reliance on direnv for first runs
joshuarli Aug 21, 2025
1dc7bb6
[skip ci] fixes
joshuarli Aug 21, 2025
bb87254
devenv should not build
joshuarli Aug 21, 2025
6388fec
simplify make setup-venv
joshuarli Aug 21, 2025
8ffdff4
install rustup via brew and add rustfmt to pre-commit
joshuarli Aug 28, 2025
320b3f8
librdkafka and update readme
joshuarli Aug 28, 2025
67826ad
exercise build_library
joshuarli Aug 29, 2025
74ff9cd
[skip ci] add note
joshuarli Aug 29, 2025
be53798
Merge branch 'master' into devenv-and-pre-commit
joshuarli Aug 29, 2025
38369f3
fix: uv is already aware of plat
joshuarli Sep 4, 2025
55e3b65
add wheel and revert to using python setup.py, uv build doesnt seem r…
joshuarli Sep 4, 2025
ee67f9c
revert to using legacy setup.py for milksnake
joshuarli Sep 29, 2025
2781325
Merge remote-tracking branch 'origin' into devenv-and-pre-commit
joshuarli Sep 29, 2025
f40e886
add build deps
joshuarli Sep 29, 2025
2914803
convince setuptools its ok to use milksnake
joshuarli Sep 29, 2025
ab5c3b7
misc fixes
joshuarli Sep 29, 2025
41febe5
Merge branch 'master' into devenv-and-pre-commit
joshuarli Oct 4, 2025
a8b829b
relay-conventions/sentry-conventions 7a3f6a296fc6
joshuarli Oct 4, 2025
5a3cd2f
Merge remote-tracking branch 'origin' into devenv-and-pre-commit
joshuarli Oct 8, 2025
676835c
update deps
joshuarli Oct 8, 2025
4b19283
restore
joshuarli Oct 8, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 16 additions & 8 deletions .envrc
Original file line number Diff line number Diff line change
@@ -1,14 +1,22 @@
#!/bin/bash
#!/not/executable/bash

# Our policy is that the .envrc is entirely optional, and a user
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

devenv/sync.py and .pre-commit-config.yaml are all using .venv/bin explicitly so this policy you have here is still being respected! although you will need a global install of devenv: https://github.com/getsentry/devenv/?tab=readme-ov-file#install

(chances are and i would hope relay devs already have devenv installed as it is required for sentry development)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually well it is needed to put .devenv/bin on path so you have access to devenv's uv at .devenv/bin/uv

# without direnv should not really have a worse dev experience in any way.

# 'make' is in charge of creating and managing virtualenvs, and things like
# pytest can still be directly invoked using .venv/bin/pytest
if [[ -f "${PWD}/.env" ]]; then
dotenv
fi

PATH_add "${HOME}/.local/share/sentry-devenv/bin"

if ! source .venv/bin/activate; then
echo "!!! you have no virtualenv, run 'make setup' to fix that."
# XXX: ideally, direnv is able to export PS1 as modified by sourcing venvs
# but we'd have to patch direnv, and ".venv" isn't descriptive anyways
unset PS1
if ! command -v devenv >/dev/null; then
echo "install devenv: https://github.com/getsentry/devenv#install"
return 1
fi

PATH_add "${PWD}/.devenv/bin"

export VIRTUAL_ENV="${PWD}/.venv"
PATH_add "${PWD}/.venv/bin"

PATH_add /opt/homebrew/opt/rustup/bin
27 changes: 21 additions & 6 deletions .github/workflows/build_library.yml
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were you able to test this workflow? Any chance we will face surprises next time we want to release the library?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah good catch, I'll run it and see!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup it's totally broken haha, will fix

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works now (i checked the wheels too): https://github.com/getsentry/relay/actions/runs/18236667618

I had to revert everything related to trying to use modern build backends unfortunately, I couldn't get it to work. legacy setuptools forever i guess, haha

Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name: Library Release Build

on:
push:
pull_request:
branches:
- release-library/**

Expand Down Expand Up @@ -69,13 +69,20 @@ jobs:
rustup override set stable
rustup target add --toolchain stable ${{ matrix.target }}

- uses: actions/setup-python@v6
- uses: astral-sh/setup-uv@884ad927a57e558e7a70b92f2bccf9198a4be546 # v6
with:
python-version: "3.10"
version: '0.8.2'
# we just cache the venv-dir directly in action-setup-venv
enable-cache: false

- uses: getsentry/action-setup-venv@3a832a9604b3e1a4202ae559248f26867b467cc7 # v2.1.1
with:
python-version: 3.11.9
cache-dependency-path: uv.lock
install-cmd: uv sync --frozen --only-dev --active

- name: Build Wheel
run: |
pip install wheel
python setup.py bdist_wheel -p ${{ matrix.py-platform }}
working-directory: py
env:
Expand All @@ -99,9 +106,17 @@ jobs:
with:
submodules: recursive

- uses: actions/setup-python@v6
- uses: astral-sh/setup-uv@884ad927a57e558e7a70b92f2bccf9198a4be546 # v6
with:
version: '0.8.2'
# we just cache the venv-dir directly in action-setup-venv
enable-cache: false

- uses: getsentry/action-setup-venv@3a832a9604b3e1a4202ae559248f26867b467cc7 # v2.1.1
with:
python-version: "3.10"
python-version: 3.11.9
cache-dependency-path: uv.lock
install-cmd: uv sync --frozen --only-dev --active

- name: Build sdist
run: python setup.py sdist
Expand Down
99 changes: 76 additions & 23 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,16 +39,21 @@ jobs:
with:
submodules: recursive

- name: Get changed files
id: changes
uses: dorny/paths-filter@0bc4621a3135347011ad047f9ecf449bf72ce2bd # v3.0.0
with:
list-files: json
filters: |
all:
- added|modified: '**/*'

- name: Setup SSH agent
if: env.SSH_PRIVATE_KEY != ''
uses: webfactory/[email protected]
with:
ssh-private-key: ${{ env.SSH_PRIVATE_KEY }}

- uses: actions/setup-python@v6
with:
python-version: "3.11"

- name: Install Rust Toolchain
run: |
rustup toolchain install stable --profile minimal --no-self-update
Expand All @@ -58,6 +63,32 @@ jobs:
with:
key: ${{ github.job }}

- uses: astral-sh/setup-uv@884ad927a57e558e7a70b92f2bccf9198a4be546 # v6
with:
version: '0.8.2'
# we just cache the venv-dir directly in action-setup-venv
enable-cache: false

- uses: getsentry/action-setup-venv@3a832a9604b3e1a4202ae559248f26867b467cc7 # v2.1.1
with:
python-version: 3.11.9
cache-dependency-path: uv.lock
install-cmd: uv sync --frozen --only-dev --active

- uses: actions/cache@1bd1e32a3bdc45362d1e726936510720a7c30a57 # v4.2.0
with:
path: ~/.cache/pre-commit
key: cache-epoch-1|${{ env.pythonLocation }}|${{ hashFiles('.pre-commit-config.yaml', 'uv.lock') }}

- name: Setup pre-commit
run: pre-commit install-hooks

- name: Run pre-commit
run: |
jq '.[]' --raw-output <<< '${{steps.changes.outputs.all_files}}' |
# Run pre-commit to lint and format check files that were changed (but not deleted)
xargs pre-commit run --files

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: CI Fails on Empty Changes; Redundant Rust Checks

The CI's pre-commit step fails when no files are changed, breaking builds for commits with only deletions or no relevant changes. It also runs make style lint redundantly for Rust checks, which pre-commit already handles, potentially causing developer confusion.

Fix in Cursor Fix in Web

- run: make style lint

- name: Check Docs
Expand Down Expand Up @@ -214,19 +245,24 @@ jobs:
- name: Install Rust Toolchain
run: rustup toolchain install stable --profile minimal --no-self-update

- uses: actions/setup-python@v6
- uses: swatinem/rust-cache@v2
with:
python-version: "3.10"
key: ${{ github.job }}

- name: Install Dependencies
run: pip install -U pytest
- uses: astral-sh/setup-uv@884ad927a57e558e7a70b92f2bccf9198a4be546 # v6
with:
version: '0.8.2'
# we just cache the venv-dir directly in action-setup-venv
enable-cache: false

- uses: swatinem/rust-cache@v2
- uses: getsentry/action-setup-venv@3a832a9604b3e1a4202ae559248f26867b467cc7 # v2.1.1
with:
key: ${{ github.job }}
python-version: 3.11.9
cache-dependency-path: uv.lock
install-cmd: uv sync --frozen --only-dev --active

- name: Build and Install Library
run: pip install -v --editable py
run: uv sync --frozen --verbose --active
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Editable Install Fails in CI

The test_python job's "Build and Install Library" step uses uv sync, which doesn't install the py/ package in editable mode. This prevents the Python library from being built, causing pytest to fail as the sentry_relay module is not found. This uv sync command is also redundant.

Fix in Cursor Fix in Web

env:
RELAY_DEBUG: 1

Expand Down Expand Up @@ -749,9 +785,17 @@ jobs:
with:
ssh-private-key: ${{ env.SSH_PRIVATE_KEY }}

- uses: actions/setup-python@v6
- uses: astral-sh/setup-uv@884ad927a57e558e7a70b92f2bccf9198a4be546 # v6
with:
python-version: "3.11"
version: '0.8.2'
# we just cache the venv-dir directly in action-setup-venv
enable-cache: false

- uses: getsentry/action-setup-venv@3a832a9604b3e1a4202ae559248f26867b467cc7 # v2.1.1
with:
python-version: 3.11.9
cache-dependency-path: uv.lock
install-cmd: uv sync --frozen --only-dev --active

- run: make test-integration
env:
Expand Down Expand Up @@ -839,13 +883,22 @@ jobs:
docker compose logs

validate-devservices-config:
runs-on: ubuntu-24.04
needs: devservices-files-changed
if: needs.devservices-files-changed.outputs.devservices-files-changed == 'true'
steps:
- uses: actions/checkout@ff7abcd0c3c05ccf6adc123a8cd1fd4fb30fb493 # v4.1.7
name: Checkout repository
- uses: getsentry/action-validate-devservices-config@02a078d1280293e6598cabfbd318a01609c12c83
name: Validate devservices config
with:
requirements-file-path: requirements-dev.txt
runs-on: ubuntu-24.04
needs: devservices-files-changed
if: needs.devservices-files-changed.outputs.devservices-files-changed == 'true'
steps:
- uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
name: Checkout repository

- name: Get devservices version
id: get-devservices-version
run: |
awk -F'"' '
/name/ { pkg = $2 }
/version/ { if (pkg == "devservices") print "version="$2 }
' uv.lock >> $GITHUB_OUTPUT

- uses: getsentry/action-validate-devservices-config@711ae7221998ddf81211f25f5e3873ecffd22387
name: Validate devservices config
with:
devservices-version: ${{ steps.get-devservices-version.outputs.version }}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: awk Script Fails to Robustly Parse uv.lock

The awk script in the validate-devservices-config job attempts to extract the devservices package version from uv.lock. However, its parsing logic is fragile, making assumptions about the order and scope of name and version fields within the TOML-like uv.lock format. This can result in the script extracting an incorrect version or failing to find it.

Fix in Cursor Fix in Web

37 changes: 36 additions & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,39 @@ repos:
rev: 0.16.0
hooks:
- id: check-github-actions
- id: check-github-workflows
# TODO: reactivate when fixed
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this referring to? What needs to be fixed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://gist.github.com/joshuarli/36587aeb18445bf18f36abcd4b82382e
(upgraded to 0.33.0 since in previous versions output is a lot less readable)

mostly complaining about 'uses' is a required property which... it is not? lol

# - id: check-github-workflows

- repo: local
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pre-commit now runs in ci and now runs black/flake8/mypy (locked+installed via uv) on python files!

hooks:
# Configuration for black exists in pyproject.toml,
# but we let pre-commit take care of the file filtering.
- id: black
name: black
entry: .venv/bin/black
language: system
types_or: [python, pyi]
require_serial: true
# Configuration for flake8 exists in setup.cfg,
# but we let pre-commit take care of the file filtering.
- id: flake8
name: flake8
entry: .venv/bin/flake8
language: system
types: [python]
require_serial: true
- id: mypy
name: mypy
entry: .venv/bin/mypy
language: system
types: [python]
require_serial: true
- id: rustfmt
name: format rust code
language: system
types: [rust]
# keep edition in sync with Cargo.toml
# There is no way to run cargo fmt for specific files,
# rustfmt can do it but doesn't read Cargo.toml.
# https://github.com/rust-lang/rustfmt/issues/4485
entry: rustfmt --edition=2024
3 changes: 3 additions & 0 deletions Brewfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
brew 'librdkafka'

brew 'rustup'
47 changes: 7 additions & 40 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
SHELL=/bin/bash
export RELAY_PYTHON_VERSION := python3
export RELAY_FEATURES :=
RELAY_CARGO_ARGS ?= ${CARGO_ARGS}

Expand Down Expand Up @@ -87,33 +86,24 @@ doc-rust: setup-git ## generate API docs for Rust code

# Style checking

style: style-rust style-python ## check code style
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style and lint for python's moved into pre-commit

i decided to leave rust alone for now to keep things simple (will need some work as tools like rustfmt and clippy-driver are needed to accept individual file arguments)

style: style-rust ## check code style
.PHONY: style

style-rust: ## check Rust code style
@rustup component add rustfmt --toolchain stable 2> /dev/null
cargo +stable fmt --all -- --check
.PHONY: style-rust

style-python: setup-venv ## check Python code style
.venv/bin/black --check py tests --exclude '\.eggs|sentry_relay/_lowlevel.*'
.PHONY: style-python

# Linting

lint: lint-rust lint-python ## runt lint on Python and Rust code
lint: lint-rust ## run lint on Rust code
.PHONY: lint

lint-rust: setup-git ## run lint on Rust code using clippy
@rustup component add clippy --toolchain stable 2> /dev/null
cargo +stable clippy --workspace --all-targets --all-features --no-deps -- -D warnings
.PHONY: lint-rust

lint-python: setup-venv ## run lint on Python code using flake8
.venv/bin/flake8 py tests
.venv/bin/mypy py tests
.PHONY: lint-python

lint-rust-beta: setup-git ## run lint on Rust using clippy and beta toolchain
@rustup toolchain install beta 2>/dev/null
@rustup component add clippy --toolchain beta 2> /dev/null
Expand All @@ -122,18 +112,14 @@ lint-rust-beta: setup-git ## run lint on Rust using clippy and beta toolchain

# Formatting

format: format-rust format-python ## format all the Rust and Python code
format: format-rust ## format all the Rust and Python code
.PHONY: format

format-rust: ## format the Rust code
@rustup component add rustfmt --toolchain stable 2> /dev/null
cargo +stable fmt --all
.PHONY: format-rust

format-python: setup-venv ## format the Python code
.venv/bin/black py tests scripts --exclude '\.eggs|sentry_relay/_lowlevel.*'
.PHONY: format-python

# Development

setup: setup-git setup-venv ## run setup tasks to create and configure development environment
Expand All @@ -143,10 +129,12 @@ init-submodules:
@git submodule update --init --recursive
.PHONY: init-submodules

setup-git: .git/hooks/pre-commit init-submodules ## make sure all git configured and all the submodules are fetched
setup-git: init-submodules ## make sure all git configured and all the submodules are fetched
.PHONY: setup-git

setup-venv: .venv/bin/python .venv/python-requirements-stamp ## create a Python virtual environment with development requirements installed
setup-venv: uv.lock ## create a Python virtual environment with development requirements installed
devenv sync
RELAY_DEBUG=1 uv pip install -v -e py
.PHONY: setup-venv

clean-target-dir:
Expand All @@ -155,27 +143,6 @@ clean-target-dir:
fi
.PHONY: clean-target-dir

# Dependencies

.venv/bin/python: Makefile
rm -rf .venv

@# --copies is necessary because OS X make checks the mtime of the symlink
@# target (/usr/local/bin/python), which is always much older than the
@# Makefile, and then proceeds to unconditionally rebuild the venv.
$$RELAY_PYTHON_VERSION -m venv --copies .venv
.venv/bin/pip install -U pip wheel

.venv/python-requirements-stamp: requirements-dev.txt
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rerunning make setup-venv will always make sure python stuff is up to date and reexecutes the build - all this complexity can be removed

.venv/bin/pip install -U -r requirements-dev.txt
RELAY_DEBUG=1 .venv/bin/pip install -v --editable py
# Bump the mtime of an empty file.
# Make will re-run 'pip install' if the mtime on requirements-dev.txt is higher again.
touch .venv/python-requirements-stamp

.git/hooks/pre-commit:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this has been rolled up into pre-commit install --install-hooks in devenv sync

@cd .git/hooks && ln -sf ../../scripts/git-precommit-hook pre-commit

help: ## this help
@ awk 'BEGIN {FS = ":.*##"; printf "Usage: make \033[36m<target>\033[0m\n\nTargets:\n"} /^[a-zA-Z_-]+:.*?##/ { printf " \033[36m%-10s\033[0m\t%s\n", $$1, $$2 }' $(MAKEFILE_LIST) | column -s$$'\t' -t
.PHONY: help
Expand Down
Loading
Loading