8000 Add devenv-requirements.txt and update env setup instructions by arr-ee · Pull Request #2761 · getsentry/sentry-python · GitHub
[go: up one dir, main page]

Skip to content

Add devenv-requirements.txt and update env setup instructions #2761

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 4, 2024

Conversation

arr-ee
Copy link
Contributor
@arr-ee arr-ee commented Feb 25, 2024

This is a quick fix for #2760.

Ideally we'd do away with constraints duplication/introduction in tox.ini, but I don't see a way to do so without much more invasive changes, and I would prefer to not make those before discussing approaches.


General Notes

Thank you for contributing to sentry-python!

Please add tests to validate your changes, and lint your code using tox -e linters.

Running the test suite on your PR might require maintainer approval. Some tests (AWS Lambda) additionally require a maintainer to add a special label to run and will fail if the label is not present.

For maintainers

Sensitive test suites require maintainer review to ensure that tests do not compromise our secrets. This review must be repeated after any code revisions.

Before running sensitive test suites, please carefully check the PR. Then, apply the Trigger: tests using secrets label. The label will be removed after any code changes to enforce our policy requiring maintainers to review all code revisions before running sensitive tests.

@antonpirker
Copy link
Member

Hey @arr-ee !
First, sorry for taking so long for reviewing this.

Thank you so much for this PR! Our devenv needs really some love :-)

One thing, there is also a mention of linter-requirements.txt in tox.ini. (which is run when you run make lint on your local machine. Could you update your PR so this still works? Thanks!

@antonpirker antonpirker self-assigned this Mar 28, 2024
This is a quick fix for getsentry#2760.

It's far from ideal, but it's quick.
@arr-ee
Copy link
Contributor Author
arr-ee commented Apr 2, 2024

One thing, there is also a mention of linter-requirements.txt in tox.ini. (which is run when you run make lint on your local machine. Could you update your PR so this still works? Thanks!

This PR does not remove linter-requirements.txt. devenv-requirements.txt just installs both test and linter deps, as well as adding constraints on pytest & friends, and packages required for integration tests that get enabled by linter requirements (mongo). tox should not be affected by this change in any way.

@arr-ee arr-ee force-pushed the devenv-requirements branch from d0fc8e8 to 95a6403 Compare April 2, 2024 14:27
@arr-ee
Copy link
Contributor Author
arr-ee commented Apr 2, 2024

db test failure is unrelated:

[17](https://github.com/getsentry/sentry-python/actions/runs/8524540156/job/23349361343#step:6:18)
Unable to find image 'clickhouse/clickhouse-server:latest' locally
[18](https://github.com/getsentry/sentry-python/actions/runs/8524540156/job/23349361343#step:6:19)
docker: Error response from daemon: received unexpected HTTP status: 503 Service Unavailable.

@antonpirker
Copy link
Member

Thanks for the clarification, makes sense :-)

As soon as the flaky test passes, this will be merged! Thanks again @arr-ee !

@antonpirker antonpirker enabled auto-merge (squash) April 4, 2024 09:18
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.

2 participants
0