Remove outdated pins - #2690

Merged
sentrivana merged 63 commits into
sentry-sdk-2.0from
ivana/2.0/update-tooling
Apr 10, 2024
Merged

Remove outdated pins#2690
sentrivana merged 63 commits into
sentry-sdk-2.0from
ivana/2.0/update-tooling

Conversation

@sentrivana

@sentrivanasentrivana commented Jan 30, 2024

Copy link
Copy Markdown
Contributor

Remove version pins on tools that are no longer necessary.

Relates #2586


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.

@sentrivanasentrivana self-assigned this Jan 30, 2024
@sentrivanasentrivana linked an issue Feb 5, 2024 that may be closed by this pull request
@sentrivanasentrivana changed the title Remove outdated pinsRemove outdated pins from test-requirementsApr 3, 2024
@sentrivanasentrivana removed a link to an issue Apr 3, 2024
@sentrivanasentrivana mentioned this pull request Apr 3, 2024
@antonpirker

Copy link
Copy Markdown
Contributor

I just ran the tests locally and there is no error. Just the tests/integrations/django/asgi/test_asgi.py tests in the Django test suite freeze and thus the test suite times out. So something with async stuff is off.

@sentrivana

Copy link
Copy Markdown
ContributorAuthor

I just ran the tests locally and there is no error. Just the tests/integrations/django/asgi/test_asgi.py tests in the Django test suite freeze and thus the test suite times out. So something with async stuff is off.

Yeah this. Also works for me locally. Interestingly, it only fails on 3.12, but it's not some dependency issue because it installs the same versions of the same dependencies on both 3.11 (where it's working) and 3.12.

Will try to bisect a bit what is causing this.

@antonpirker

Copy link
Copy Markdown
Contributor

I just messed around with the Django tests. The problem is the following:

  • We have this "Web Frameworks 1" test suite that runs multiple test suites in a matrix.
  • Every test suite uses the service "postgres" defined in the yaml file.
  • And in this postgres service we create one database called "ci_test"
  • When the first test that is decorator with pytest_mark_django_db_decorator is run, the database is filled by running the django migrations.

And my guess now is that when two test suites run at the same time they both want to fill the db by running the migrations and because there is already something in the db, it fails with the duplicate key value violates unique constraint error.

And as so often when fixing bugs the question is: How has this ever worked at some point? :-)

Solution would now be to either have one postgres server by test suite, or one unique database name per test suite. (or maybe create the postgres server in some completely different way)

@antonpirker

Copy link
Copy Markdown
Contributor

I played around more and have this: https://github.com/getsentry/sentry-python/actions/runs/8572070273/job/23493776173?pr=2938

The thing now is that in "Setup Test Env" i try to delete the uniquely named db name but it does not exist. (which is fine) Two steps later when running the django tests they fail becaus the db is already existing. So the db is not cleaned up between running different django versions in one call to tox. Not sure why, but this is something for future Anton!

@sentrivana

Copy link
Copy Markdown
ContributorAuthor

@antonpirker Maybe this can be somehow combined with --create-db/--reuse-db? https://pytest-django.readthedocs.io/en/latest/database.html#create-db-force-re-creation-of-the-test-database

--create-db - force re creation of the test database
When used with --reuse-db, this option will re-create the database, regardless of whether it exists or not.

@antonpirker

Copy link
Copy Markdown
Contributor

I think I have solved the problem now in my PR: #2938
I removed the whole setting of the db name in CI with env variables and just let the django app itself set the database name (including a random number) this way we always have one db with a unique name for a testrun that is also cleaned up by pytest-django. if the tests pass, I will merge my PR into this one

@antonpirker

Copy link
Copy Markdown
Contributor

Ok, the Python 3.12 tests still freeze, but the rest is green so I will merge it into here.

@sentrivana

Copy link
Copy Markdown
ContributorAuthor

Awesome! The Web Frameworks 1 tests on 3.12 will still time out but that's not a problem with your PR. Everything else seems green so feel free to merge

antonpirkerand others added 6 commits April 8, 2024 11:25
Make sure the Django tests are always run with a unique database name that is cleaned up after the tests have been run.
@sentrivana

sentrivana commented Apr 8, 2024

Copy link
Copy Markdown
ContributorAuthor

Regarding the freezing 3.12 tests: the problem goes away as soon as you remove tox from test-requirements.txt. Which I think we can do -- the purpose of test-requirements seems to be to set up the test env after tox has already created it.

@sentrivana
sentrivana marked this pull request as ready for review April 8, 2024 10:13
@sentrivana

Copy link
Copy Markdown
ContributorAuthor

Marking this as ready for review but we shouldn't merge this until the final release is out.

@sentrivanasentrivana linked an issue Apr 8, 2024 that may be closed by this pull request
@sentrivana
sentrivana merged commit 467bde9 into sentry-sdk-2.0Apr 10, 2024
@sentrivana
sentrivana deleted the ivana/2.0/update-tooling branch April 10, 2024 07:37
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update tooling

2 participants

@sentrivana@antonpirker
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all \u003cpre\u003e\u003ccode\u003e blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks"); } } catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); } })(); (function(){ try { var __m = "github.com"; var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Remove outdated pins - #2690

Merged
sentrivana merged 63 commits into
sentry-sdk-2.0from
ivana/2.0/update-tooling
Apr 10, 2024
Merged

Remove outdated pins#2690
sentrivana merged 63 commits into
sentry-sdk-2.0from
ivana/2.0/update-tooling

Conversation

@sentrivana

@sentrivanasentrivana commented Jan 30, 2024

Copy link
Copy Markdown
Contributor

Remove version pins on tools that are no longer necessary.

Relates #2586


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.

@sentrivanasentrivana self-assigned this Jan 30, 2024
@sentrivanasentrivana linked an issue Feb 5, 2024 that may be closed by this pull request
@sentrivanasentrivana changed the title Remove outdated pinsRemove outdated pins from test-requirementsApr 3, 2024
@sentrivanasentrivana removed a link to an issue Apr 3, 2024
@sentrivanasentrivana mentioned this pull request Apr 3, 2024
@antonpirker

Copy link
Copy Markdown
Contributor

I just ran the tests locally and there is no error. Just the tests/integrations/django/asgi/test_asgi.py tests in the Django test suite freeze and thus the test suite times out. So something with async stuff is off.

@sentrivana

Copy link
Copy Markdown
ContributorAuthor

I just ran the tests locally and there is no error. Just the tests/integrations/django/asgi/test_asgi.py tests in the Django test suite freeze and thus the test suite times out. So something with async stuff is off.

Yeah this. Also works for me locally. Interestingly, it only fails on 3.12, but it's not some dependency issue because it installs the same versions of the same dependencies on both 3.11 (where it's working) and 3.12.

Will try to bisect a bit what is causing this.

@antonpirker

Copy link
Copy Markdown
Contributor

I just messed around with the Django tests. The problem is the following:

  • We have this "Web Frameworks 1" test suite that runs multiple test suites in a matrix.
  • Every test suite uses the service "postgres" defined in the yaml file.
  • And in this postgres service we create one database called "ci_test"
  • When the first test that is decorator with pytest_mark_django_db_decorator is run, the database is filled by running the django migrations.

And my guess now is that when two test suites run at the same time they both want to fill the db by running the migrations and because there is already something in the db, it fails with the duplicate key value violates unique constraint error.

And as so often when fixing bugs the question is: How has this ever worked at some point? :-)

Solution would now be to either have one postgres server by test suite, or one unique database name per test suite. (or maybe create the postgres server in some completely different way)

@antonpirker

Copy link
Copy Markdown
Contributor

I played around more and have this: https://github.com/getsentry/sentry-python/actions/runs/8572070273/job/23493776173?pr=2938

The thing now is that in "Setup Test Env" i try to delete the uniquely named db name but it does not exist. (which is fine) Two steps later when running the django tests they fail becaus the db is already existing. So the db is not cleaned up between running different django versions in one call to tox. Not sure why, but this is something for future Anton!

@sentrivana

Copy link
Copy Markdown
ContributorAuthor

@antonpirker Maybe this can be somehow combined with --create-db/--reuse-db? https://pytest-django.readthedocs.io/en/latest/database.html#create-db-force-re-creation-of-the-test-database

--create-db - force re creation of the test database
When used with --reuse-db, this option will re-create the database, regardless of whether it exists or not.

@antonpirker

Copy link
Copy Markdown
Contributor

I think I have solved the problem now in my PR: #2938
I removed the whole setting of the db name in CI with env variables and just let the django app itself set the database name (including a random number) this way we always have one db with a unique name for a testrun that is also cleaned up by pytest-django. if the tests pass, I will merge my PR into this one

@antonpirker

Copy link
Copy Markdown
Contributor

Ok, the Python 3.12 tests still freeze, but the rest is green so I will merge it into here.

@sentrivana

Copy link
Copy Markdown
ContributorAuthor

Awesome! The Web Frameworks 1 tests on 3.12 will still time out but that's not a problem with your PR. Everything else seems green so feel free to merge

antonpirkerand others added 6 commits April 8, 2024 11:25
Make sure the Django tests are always run with a unique database name that is cleaned up after the tests have been run.
@sentrivana

sentrivana commented Apr 8, 2024

Copy link
Copy Markdown
ContributorAuthor

Regarding the freezing 3.12 tests: the problem goes away as soon as you remove tox from test-requirements.txt. Which I think we can do -- the purpose of test-requirements seems to be to set up the test env after tox has already created it.

@sentrivana
sentrivana marked this pull request as ready for review April 8, 2024 10:13
@sentrivana

Copy link
Copy Markdown
ContributorAuthor

Marking this as ready for review but we shouldn't merge this until the final release is out.

@sentrivanasentrivana linked an issue Apr 8, 2024 that may be closed by this pull request
@sentrivana
sentrivana merged commit 467bde9 into sentry-sdk-2.0Apr 10, 2024
@sentrivana
sentrivana deleted the ivana/2.0/update-tooling branch April 10, 2024 07:37
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update tooling

2 participants

@sentrivana@antonpirker
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Remove outdated pins - #2690

Merged
sentrivana merged 63 commits into
sentry-sdk-2.0from
ivana/2.0/update-tooling
Apr 10, 2024
Merged

Remove outdated pins#2690
sentrivana merged 63 commits into
sentry-sdk-2.0from
ivana/2.0/update-tooling

Conversation

@sentrivana

@sentrivanasentrivana commented Jan 30, 2024

Copy link
Copy Markdown
Contributor

Remove version pins on tools that are no longer necessary.

Relates #2586


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.

@sentrivanasentrivana self-assigned this Jan 30, 2024
@sentrivanasentrivana linked an issue Feb 5, 2024 that may be closed by this pull request
@sentrivanasentrivana changed the title Remove outdated pinsRemove outdated pins from test-requirementsApr 3, 2024
@sentrivanasentrivana removed a link to an issue Apr 3, 2024
@sentrivanasentrivana mentioned this pull request Apr 3, 2024
@antonpirker

Copy link
Copy Markdown
Contributor

I just ran the tests locally and there is no error. Just the tests/integrations/django/asgi/test_asgi.py tests in the Django test suite freeze and thus the test suite times out. So something with async stuff is off.

@sentrivana

Copy link
Copy Markdown
ContributorAuthor

I just ran the tests locally and there is no error. Just the tests/integrations/django/asgi/test_asgi.py tests in the Django test suite freeze and thus the test suite times out. So something with async stuff is off.

Yeah this. Also works for me locally. Interestingly, it only fails on 3.12, but it's not some dependency issue because it installs the same versions of the same dependencies on both 3.11 (where it's working) and 3.12.

Will try to bisect a bit what is causing this.

@antonpirker

Copy link
Copy Markdown
Contributor

I just messed around with the Django tests. The problem is the following:

  • We have this "Web Frameworks 1" test suite that runs multiple test suites in a matrix.
  • Every test suite uses the service "postgres" defined in the yaml file.
  • And in this postgres service we create one database called "ci_test"
  • When the first test that is decorator with pytest_mark_django_db_decorator is run, the database is filled by running the django migrations.

And my guess now is that when two test suites run at the same time they both want to fill the db by running the migrations and because there is already something in the db, it fails with the duplicate key value violates unique constraint error.

And as so often when fixing bugs the question is: How has this ever worked at some point? :-)

Solution would now be to either have one postgres server by test suite, or one unique database name per test suite. (or maybe create the postgres server in some completely different way)

@antonpirker

Copy link
Copy Markdown
Contributor

I played around more and have this: https://github.com/getsentry/sentry-python/actions/runs/8572070273/job/23493776173?pr=2938

The thing now is that in "Setup Test Env" i try to delete the uniquely named db name but it does not exist. (which is fine) Two steps later when running the django tests they fail becaus the db is already existing. So the db is not cleaned up between running different django versions in one call to tox. Not sure why, but this is something for future Anton!

@sentrivana

Copy link
Copy Markdown
ContributorAuthor

@antonpirker Maybe this can be somehow combined with --create-db/--reuse-db? https://pytest-django.readthedocs.io/en/latest/database.html#create-db-force-re-creation-of-the-test-database

--create-db - force re creation of the test database
When used with --reuse-db, this option will re-create the database, regardless of whether it exists or not.

@antonpirker

Copy link
Copy Markdown
Contributor

I think I have solved the problem now in my PR: #2938
I removed the whole setting of the db name in CI with env variables and just let the django app itself set the database name (including a random number) this way we always have one db with a unique name for a testrun that is also cleaned up by pytest-django. if the tests pass, I will merge my PR into this one

@antonpirker

Copy link
Copy Markdown
Contributor

Ok, the Python 3.12 tests still freeze, but the rest is green so I will merge it into here.

@sentrivana

Copy link
Copy Markdown
ContributorAuthor

Awesome! The Web Frameworks 1 tests on 3.12 will still time out but that's not a problem with your PR. Everything else seems green so feel free to merge

antonpirkerand others added 6 commits April 8, 2024 11:25
Make sure the Django tests are always run with a unique database name that is cleaned up after the tests have been run.
@sentrivana

sentrivana commented Apr 8, 2024

Copy link
Copy Markdown
ContributorAuthor

Regarding the freezing 3.12 tests: the problem goes away as soon as you remove tox from test-requirements.txt. Which I think we can do -- the purpose of test-requirements seems to be to set up the test env after tox has already created it.

@sentrivana
sentrivana marked this pull request as ready for review April 8, 2024 10:13
@sentrivana

Copy link
Copy Markdown
ContributorAuthor

Marking this as ready for review but we shouldn't merge this until the final release is out.

@sentrivanasentrivana linked an issue Apr 8, 2024 that may be closed by this pull request
@sentrivana
sentrivana merged commit 467bde9 into sentry-sdk-2.0Apr 10, 2024
@sentrivana
sentrivana deleted the ivana/2.0/update-tooling branch April 10, 2024 07:37
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update tooling

2 participants

@sentrivana@antonpirker
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length \u003e 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Remove outdated pins - #2690

Merged
sentrivana merged 63 commits into
sentry-sdk-2.0from
ivana/2.0/update-tooling
Apr 10, 2024
Merged

Remove outdated pins#2690
sentrivana merged 63 commits into
sentry-sdk-2.0from
ivana/2.0/update-tooling

Conversation

@sentrivana

@sentrivanasentrivana commented Jan 30, 2024

Copy link
Copy Markdown
Contributor

Remove version pins on tools that are no longer necessary.

Relates #2586


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.

@sentrivanasentrivana self-assigned this Jan 30, 2024
@sentrivanasentrivana linked an issue Feb 5, 2024 that may be closed by this pull request
@sentrivanasentrivana changed the title Remove outdated pinsRemove outdated pins from test-requirementsApr 3, 2024
@sentrivanasentrivana removed a link to an issue Apr 3, 2024
@sentrivanasentrivana mentioned this pull request Apr 3, 2024
@antonpirker

Copy link
Copy Markdown
Contributor

I just ran the tests locally and there is no error. Just the tests/integrations/django/asgi/test_asgi.py tests in the Django test suite freeze and thus the test suite times out. So something with async stuff is off.

@sentrivana

Copy link
Copy Markdown
ContributorAuthor

I just ran the tests locally and there is no error. Just the tests/integrations/django/asgi/test_asgi.py tests in the Django test suite freeze and thus the test suite times out. So something with async stuff is off.

Yeah this. Also works for me locally. Interestingly, it only fails on 3.12, but it's not some dependency issue because it installs the same versions of the same dependencies on both 3.11 (where it's working) and 3.12.

Will try to bisect a bit what is causing this.

@antonpirker

Copy link
Copy Markdown
Contributor

I just messed around with the Django tests. The problem is the following:

  • We have this "Web Frameworks 1" test suite that runs multiple test suites in a matrix.
  • Every test suite uses the service "postgres" defined in the yaml file.
  • And in this postgres service we create one database called "ci_test"
  • When the first test that is decorator with pytest_mark_django_db_decorator is run, the database is filled by running the django migrations.

And my guess now is that when two test suites run at the same time they both want to fill the db by running the migrations and because there is already something in the db, it fails with the duplicate key value violates unique constraint error.

And as so often when fixing bugs the question is: How has this ever worked at some point? :-)

Solution would now be to either have one postgres server by test suite, or one unique database name per test suite. (or maybe create the postgres server in some completely different way)

@antonpirker

Copy link
Copy Markdown
Contributor

I played around more and have this: https://github.com/getsentry/sentry-python/actions/runs/8572070273/job/23493776173?pr=2938

The thing now is that in "Setup Test Env" i try to delete the uniquely named db name but it does not exist. (which is fine) Two steps later when running the django tests they fail becaus the db is already existing. So the db is not cleaned up between running different django versions in one call to tox. Not sure why, but this is something for future Anton!

@sentrivana

Copy link
Copy Markdown
ContributorAuthor

@antonpirker Maybe this can be somehow combined with --create-db/--reuse-db? https://pytest-django.readthedocs.io/en/latest/database.html#create-db-force-re-creation-of-the-test-database

--create-db - force re creation of the test database
When used with --reuse-db, this option will re-create the database, regardless of whether it exists or not.

@antonpirker

Copy link
Copy Markdown
Contributor

I think I have solved the problem now in my PR: #2938
I removed the whole setting of the db name in CI with env variables and just let the django app itself set the database name (including a random number) this way we always have one db with a unique name for a testrun that is also cleaned up by pytest-django. if the tests pass, I will merge my PR into this one

@antonpirker

Copy link
Copy Markdown
Contributor

Ok, the Python 3.12 tests still freeze, but the rest is green so I will merge it into here.

@sentrivana

Copy link
Copy Markdown
ContributorAuthor

Awesome! The Web Frameworks 1 tests on 3.12 will still time out but that's not a problem with your PR. Everything else seems green so feel free to merge

antonpirkerand others added 6 commits April 8, 2024 11:25
Make sure the Django tests are always run with a unique database name that is cleaned up after the tests have been run.
@sentrivana

sentrivana commented Apr 8, 2024

Copy link
Copy Markdown
ContributorAuthor

Regarding the freezing 3.12 tests: the problem goes away as soon as you remove tox from test-requirements.txt. Which I think we can do -- the purpose of test-requirements seems to be to set up the test env after tox has already created it.

@sentrivana
sentrivana marked this pull request as ready for review April 8, 2024 10:13
@sentrivana

Copy link
Copy Markdown
ContributorAuthor

Marking this as ready for review but we shouldn't merge this until the final release is out.

@sentrivanasentrivana linked an issue Apr 8, 2024 that may be closed by this pull request
@sentrivana
sentrivana merged commit 467bde9 into sentry-sdk-2.0Apr 10, 2024
@sentrivana
sentrivana deleted the ivana/2.0/update-tooling branch April 10, 2024 07:37
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update tooling

2 participants

@sentrivana@antonpirker
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Remove outdated pins - #2690

Merged
sentrivana merged 63 commits into
sentry-sdk-2.0from
ivana/2.0/update-tooling
Apr 10, 2024
Merged

Remove outdated pins#2690
sentrivana merged 63 commits into
sentry-sdk-2.0from
ivana/2.0/update-tooling

Conversation

@sentrivana

@sentrivanasentrivana commented Jan 30, 2024

Copy link
Copy Markdown
Contributor

Remove version pins on tools that are no longer necessary.

Relates #2586


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.

@sentrivanasentrivana self-assigned this Jan 30, 2024
@sentrivanasentrivana linked an issue Feb 5, 2024 that may be closed by this pull request
@sentrivanasentrivana changed the title Remove outdated pinsRemove outdated pins from test-requirementsApr 3, 2024
@sentrivanasentrivana removed a link to an issue Apr 3, 2024
@sentrivanasentrivana mentioned this pull request Apr 3, 2024
@antonpirker

Copy link
Copy Markdown
Contributor

I just ran the tests locally and there is no error. Just the tests/integrations/django/asgi/test_asgi.py tests in the Django test suite freeze and thus the test suite times out. So something with async stuff is off.

@sentrivana

Copy link
Copy Markdown
ContributorAuthor

I just ran the tests locally and there is no error. Just the tests/integrations/django/asgi/test_asgi.py tests in the Django test suite freeze and thus the test suite times out. So something with async stuff is off.

Yeah this. Also works for me locally. Interestingly, it only fails on 3.12, but it's not some dependency issue because it installs the same versions of the same dependencies on both 3.11 (where it's working) and 3.12.

Will try to bisect a bit what is causing this.

@antonpirker

Copy link
Copy Markdown
Contributor

I just messed around with the Django tests. The problem is the following:

  • We have this "Web Frameworks 1" test suite that runs multiple test suites in a matrix.
  • Every test suite uses the service "postgres" defined in the yaml file.
  • And in this postgres service we create one database called "ci_test"
  • When the first test that is decorator with pytest_mark_django_db_decorator is run, the database is filled by running the django migrations.

And my guess now is that when two test suites run at the same time they both want to fill the db by running the migrations and because there is already something in the db, it fails with the duplicate key value violates unique constraint error.

And as so often when fixing bugs the question is: How has this ever worked at some point? :-)

Solution would now be to either have one postgres server by test suite, or one unique database name per test suite. (or maybe create the postgres server in some completely different way)

@antonpirker

Copy link
Copy Markdown
Contributor

I played around more and have this: https://github.com/getsentry/sentry-python/actions/runs/8572070273/job/23493776173?pr=2938

The thing now is that in "Setup Test Env" i try to delete the uniquely named db name but it does not exist. (which is fine) Two steps later when running the django tests they fail becaus the db is already existing. So the db is not cleaned up between running different django versions in one call to tox. Not sure why, but this is something for future Anton!

@sentrivana

Copy link
Copy Markdown
ContributorAuthor

@antonpirker Maybe this can be somehow combined with --create-db/--reuse-db? https://pytest-django.readthedocs.io/en/latest/database.html#create-db-force-re-creation-of-the-test-database

--create-db - force re creation of the test database
When used with --reuse-db, this option will re-create the database, regardless of whether it exists or not.

@antonpirker

Copy link
Copy Markdown
Contributor

I think I have solved the problem now in my PR: #2938
I removed the whole setting of the db name in CI with env variables and just let the django app itself set the database name (including a random number) this way we always have one db with a unique name for a testrun that is also cleaned up by pytest-django. if the tests pass, I will merge my PR into this one

@antonpirker

Copy link
Copy Markdown
Contributor

Ok, the Python 3.12 tests still freeze, but the rest is green so I will merge it into here.

@sentrivana

Copy link
Copy Markdown
ContributorAuthor

Awesome! The Web Frameworks 1 tests on 3.12 will still time out but that's not a problem with your PR. Everything else seems green so feel free to merge

antonpirkerand others added 6 commits April 8, 2024 11:25
Make sure the Django tests are always run with a unique database name that is cleaned up after the tests have been run.
@sentrivana

sentrivana commented Apr 8, 2024

Copy link
Copy Markdown
ContributorAuthor

Regarding the freezing 3.12 tests: the problem goes away as soon as you remove tox from test-requirements.txt. Which I think we can do -- the purpose of test-requirements seems to be to set up the test env after tox has already created it.

@sentrivana
sentrivana marked this pull request as ready for review April 8, 2024 10:13
@sentrivana

Copy link
Copy Markdown
ContributorAuthor

Marking this as ready for review but we shouldn't merge this until the final release is out.

@sentrivanasentrivana linked an issue Apr 8, 2024 that may be closed by this pull request
@sentrivana
sentrivana merged commit 467bde9 into sentry-sdk-2.0Apr 10, 2024
@sentrivana
sentrivana deleted the ivana/2.0/update-tooling branch April 10, 2024 07:37
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update tooling

2 participants

@sentrivana@antonpirker
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Remove outdated pins - #2690

Merged
sentrivana merged 63 commits into
sentry-sdk-2.0from
ivana/2.0/update-tooling
Apr 10, 2024
Merged

Remove outdated pins#2690
sentrivana merged 63 commits into
sentry-sdk-2.0from
ivana/2.0/update-tooling

Conversation

@sentrivana

@sentrivanasentrivana commented Jan 30, 2024

Copy link
Copy Markdown
Contributor

Remove version pins on tools that are no longer necessary.

Relates #2586


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.

@sentrivanasentrivana self-assigned this Jan 30, 2024
@sentrivanasentrivana linked an issue Feb 5, 2024 that may be closed by this pull request
@sentrivanasentrivana changed the title Remove outdated pinsRemove outdated pins from test-requirementsApr 3, 2024
@sentrivanasentrivana removed a link to an issue Apr 3, 2024
@sentrivanasentrivana mentioned this pull request Apr 3, 2024
@antonpirker

Copy link
Copy Markdown
Contributor

I just ran the tests locally and there is no error. Just the tests/integrations/django/asgi/test_asgi.py tests in the Django test suite freeze and thus the test suite times out. So something with async stuff is off.

@sentrivana

Copy link
Copy Markdown
ContributorAuthor

I just ran the tests locally and there is no error. Just the tests/integrations/django/asgi/test_asgi.py tests in the Django test suite freeze and thus the test suite times out. So something with async stuff is off.

Yeah this. Also works for me locally. Interestingly, it only fails on 3.12, but it's not some dependency issue because it installs the same versions of the same dependencies on both 3.11 (where it's working) and 3.12.

Will try to bisect a bit what is causing this.

@antonpirker

Copy link
Copy Markdown
Contributor

I just messed around with the Django tests. The problem is the following:

  • We have this "Web Frameworks 1" test suite that runs multiple test suites in a matrix.
  • Every test suite uses the service "postgres" defined in the yaml file.
  • And in this postgres service we create one database called "ci_test"
  • When the first test that is decorator with pytest_mark_django_db_decorator is run, the database is filled by running the django migrations.

And my guess now is that when two test suites run at the same time they both want to fill the db by running the migrations and because there is already something in the db, it fails with the duplicate key value violates unique constraint error.

And as so often when fixing bugs the question is: How has this ever worked at some point? :-)

Solution would now be to either have one postgres server by test suite, or one unique database name per test suite. (or maybe create the postgres server in some completely different way)

@antonpirker

Copy link
Copy Markdown
Contributor

I played around more and have this: https://github.com/getsentry/sentry-python/actions/runs/8572070273/job/23493776173?pr=2938

The thing now is that in "Setup Test Env" i try to delete the uniquely named db name but it does not exist. (which is fine) Two steps later when running the django tests they fail becaus the db is already existing. So the db is not cleaned up between running different django versions in one call to tox. Not sure why, but this is something for future Anton!

@sentrivana

Copy link
Copy Markdown
ContributorAuthor

@antonpirker Maybe this can be somehow combined with --create-db/--reuse-db? https://pytest-django.readthedocs.io/en/latest/database.html#create-db-force-re-creation-of-the-test-database

--create-db - force re creation of the test database
When used with --reuse-db, this option will re-create the database, regardless of whether it exists or not.

@antonpirker

Copy link
Copy Markdown
Contributor

I think I have solved the problem now in my PR: #2938
I removed the whole setting of the db name in CI with env variables and just let the django app itself set the database name (including a random number) this way we always have one db with a unique name for a testrun that is also cleaned up by pytest-django. if the tests pass, I will merge my PR into this one

@antonpirker

Copy link
Copy Markdown
Contributor

Ok, the Python 3.12 tests still freeze, but the rest is green so I will merge it into here.

@sentrivana

Copy link
Copy Markdown
ContributorAuthor

Awesome! The Web Frameworks 1 tests on 3.12 will still time out but that's not a problem with your PR. Everything else seems green so feel free to merge

antonpirkerand others added 6 commits April 8, 2024 11:25
Make sure the Django tests are always run with a unique database name that is cleaned up after the tests have been run.
@sentrivana

sentrivana commented Apr 8, 2024

Copy link
Copy Markdown
ContributorAuthor

Regarding the freezing 3.12 tests: the problem goes away as soon as you remove tox from test-requirements.txt. Which I think we can do -- the purpose of test-requirements seems to be to set up the test env after tox has already created it.

@sentrivana
sentrivana marked this pull request as ready for review April 8, 2024 10:13
@sentrivana

Copy link
Copy Markdown
ContributorAuthor

Marking this as ready for review but we shouldn't merge this until the final release is out.

@sentrivanasentrivana linked an issue Apr 8, 2024 that may be closed by this pull request
@sentrivana
sentrivana merged commit 467bde9 into sentry-sdk-2.0Apr 10, 2024
@sentrivana
sentrivana deleted the ivana/2.0/update-tooling branch April 10, 2024 07:37
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update tooling

2 participants

@sentrivana@antonpirker
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Remove outdated pins - #2690

Merged
sentrivana merged 63 commits into
sentry-sdk-2.0from
ivana/2.0/update-tooling
Apr 10, 2024
Merged

Remove outdated pins#2690
sentrivana merged 63 commits into
sentry-sdk-2.0from
ivana/2.0/update-tooling

Conversation

@sentrivana

@sentrivanasentrivana commented Jan 30, 2024

Copy link
Copy Markdown
Contributor

Remove version pins on tools that are no longer necessary.

Relates #2586


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.

@sentrivanasentrivana self-assigned this Jan 30, 2024
@sentrivanasentrivana linked an issue Feb 5, 2024 that may be closed by this pull request
@sentrivanasentrivana changed the title Remove outdated pinsRemove outdated pins from test-requirementsApr 3, 2024
@sentrivanasentrivana removed a link to an issue Apr 3, 2024
@sentrivanasentrivana mentioned this pull request Apr 3, 2024
@antonpirker

Copy link
Copy Markdown
Contributor

I just ran the tests locally and there is no error. Just the tests/integrations/django/asgi/test_asgi.py tests in the Django test suite freeze and thus the test suite times out. So something with async stuff is off.

@sentrivana

Copy link
Copy Markdown
ContributorAuthor

I just ran the tests locally and there is no error. Just the tests/integrations/django/asgi/test_asgi.py tests in the Django test suite freeze and thus the test suite times out. So something with async stuff is off.

Yeah this. Also works for me locally. Interestingly, it only fails on 3.12, but it's not some dependency issue because it installs the same versions of the same dependencies on both 3.11 (where it's working) and 3.12.

Will try to bisect a bit what is causing this.

@antonpirker

Copy link
Copy Markdown
Contributor

I just messed around with the Django tests. The problem is the following:

  • We have this "Web Frameworks 1" test suite that runs multiple test suites in a matrix.
  • Every test suite uses the service "postgres" defined in the yaml file.
  • And in this postgres service we create one database called "ci_test"
  • When the first test that is decorator with pytest_mark_django_db_decorator is run, the database is filled by running the django migrations.

And my guess now is that when two test suites run at the same time they both want to fill the db by running the migrations and because there is already something in the db, it fails with the duplicate key value violates unique constraint error.

And as so often when fixing bugs the question is: How has this ever worked at some point? :-)

Solution would now be to either have one postgres server by test suite, or one unique database name per test suite. (or maybe create the postgres server in some completely different way)

@antonpirker

Copy link
Copy Markdown
Contributor

I played around more and have this: https://github.com/getsentry/sentry-python/actions/runs/8572070273/job/23493776173?pr=2938

The thing now is that in "Setup Test Env" i try to delete the uniquely named db name but it does not exist. (which is fine) Two steps later when running the django tests they fail becaus the db is already existing. So the db is not cleaned up between running different django versions in one call to tox. Not sure why, but this is something for future Anton!

@sentrivana

Copy link
Copy Markdown
ContributorAuthor

@antonpirker Maybe this can be somehow combined with --create-db/--reuse-db? https://pytest-django.readthedocs.io/en/latest/database.html#create-db-force-re-creation-of-the-test-database

--create-db - force re creation of the test database
When used with --reuse-db, this option will re-create the database, regardless of whether it exists or not.

@antonpirker

Copy link
Copy Markdown
Contributor

I think I have solved the problem now in my PR: #2938
I removed the whole setting of the db name in CI with env variables and just let the django app itself set the database name (including a random number) this way we always have one db with a unique name for a testrun that is also cleaned up by pytest-django. if the tests pass, I will merge my PR into this one

@antonpirker

Copy link
Copy Markdown
Contributor

Ok, the Python 3.12 tests still freeze, but the rest is green so I will merge it into here.

@sentrivana

Copy link
Copy Markdown
ContributorAuthor

Awesome! The Web Frameworks 1 tests on 3.12 will still time out but that's not a problem with your PR. Everything else seems green so feel free to merge

antonpirkerand others added 6 commits April 8, 2024 11:25
Make sure the Django tests are always run with a unique database name that is cleaned up after the tests have been run.
@sentrivana

sentrivana commented Apr 8, 2024

Copy link
Copy Markdown
ContributorAuthor

Regarding the freezing 3.12 tests: the problem goes away as soon as you remove tox from test-requirements.txt. Which I think we can do -- the purpose of test-requirements seems to be to set up the test env after tox has already created it.

@sentrivana
sentrivana marked this pull request as ready for review April 8, 2024 10:13
@sentrivana

Copy link
Copy Markdown
ContributorAuthor

Marking this as ready for review but we shouldn't merge this until the final release is out.

@sentrivanasentrivana linked an issue Apr 8, 2024 that may be closed by this pull request
@sentrivana
sentrivana merged commit 467bde9 into sentry-sdk-2.0Apr 10, 2024
@sentrivana
sentrivana deleted the ivana/2.0/update-tooling branch April 10, 2024 07:37
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update tooling

2 participants

@sentrivana@antonpirker
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Remove outdated pins - #2690

Merged
sentrivana merged 63 commits into
sentry-sdk-2.0from
ivana/2.0/update-tooling
Apr 10, 2024
Merged

Remove outdated pins#2690
sentrivana merged 63 commits into
sentry-sdk-2.0from
ivana/2.0/update-tooling

Conversation

@sentrivana

@sentrivanasentrivana commented Jan 30, 2024

Copy link
Copy Markdown
Contributor

Remove version pins on tools that are no longer necessary.

Relates #2586


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.

@sentrivanasentrivana self-assigned this Jan 30, 2024
@sentrivanasentrivana linked an issue Feb 5, 2024 that may be closed by this pull request
@sentrivanasentrivana changed the title Remove outdated pinsRemove outdated pins from test-requirementsApr 3, 2024
@sentrivanasentrivana removed a link to an issue Apr 3, 2024
@sentrivanasentrivana mentioned this pull request Apr 3, 2024
@antonpirker

Copy link
Copy Markdown
Contributor

I just ran the tests locally and there is no error. Just the tests/integrations/django/asgi/test_asgi.py tests in the Django test suite freeze and thus the test suite times out. So something with async stuff is off.

@sentrivana

Copy link
Copy Markdown
ContributorAuthor

I just ran the tests locally and there is no error. Just the tests/integrations/django/asgi/test_asgi.py tests in the Django test suite freeze and thus the test suite times out. So something with async stuff is off.

Yeah this. Also works for me locally. Interestingly, it only fails on 3.12, but it's not some dependency issue because it installs the same versions of the same dependencies on both 3.11 (where it's working) and 3.12.

Will try to bisect a bit what is causing this.

@antonpirker

Copy link
Copy Markdown
Contributor

I just messed around with the Django tests. The problem is the following:

  • We have this "Web Frameworks 1" test suite that runs multiple test suites in a matrix.
  • Every test suite uses the service "postgres" defined in the yaml file.
  • And in this postgres service we create one database called "ci_test"
  • When the first test that is decorator with pytest_mark_django_db_decorator is run, the database is filled by running the django migrations.

And my guess now is that when two test suites run at the same time they both want to fill the db by running the migrations and because there is already something in the db, it fails with the duplicate key value violates unique constraint error.

And as so often when fixing bugs the question is: How has this ever worked at some point? :-)

Solution would now be to either have one postgres server by test suite, or one unique database name per test suite. (or maybe create the postgres server in some completely different way)

@antonpirker

Copy link
Copy Markdown
Contributor

I played around more and have this: https://github.com/getsentry/sentry-python/actions/runs/8572070273/job/23493776173?pr=2938

The thing now is that in "Setup Test Env" i try to delete the uniquely named db name but it does not exist. (which is fine) Two steps later when running the django tests they fail becaus the db is already existing. So the db is not cleaned up between running different django versions in one call to tox. Not sure why, but this is something for future Anton!

@sentrivana

Copy link
Copy Markdown
ContributorAuthor

@antonpirker Maybe this can be somehow combined with --create-db/--reuse-db? https://pytest-django.readthedocs.io/en/latest/database.html#create-db-force-re-creation-of-the-test-database

--create-db - force re creation of the test database
When used with --reuse-db, this option will re-create the database, regardless of whether it exists or not.

@antonpirker

Copy link
Copy Markdown
Contributor

I think I have solved the problem now in my PR: #2938
I removed the whole setting of the db name in CI with env variables and just let the django app itself set the database name (including a random number) this way we always have one db with a unique name for a testrun that is also cleaned up by pytest-django. if the tests pass, I will merge my PR into this one

@antonpirker

Copy link
Copy Markdown
Contributor

Ok, the Python 3.12 tests still freeze, but the rest is green so I will merge it into here.

@sentrivana

Copy link
Copy Markdown
ContributorAuthor

Awesome! The Web Frameworks 1 tests on 3.12 will still time out but that's not a problem with your PR. Everything else seems green so feel free to merge

antonpirkerand others added 6 commits April 8, 2024 11:25
Make sure the Django tests are always run with a unique database name that is cleaned up after the tests have been run.
@sentrivana

sentrivana commented Apr 8, 2024

Copy link
Copy Markdown
ContributorAuthor

Regarding the freezing 3.12 tests: the problem goes away as soon as you remove tox from test-requirements.txt. Which I think we can do -- the purpose of test-requirements seems to be to set up the test env after tox has already created it.

@sentrivana
sentrivana marked this pull request as ready for review April 8, 2024 10:13
@sentrivana

Copy link
Copy Markdown
ContributorAuthor

Marking this as ready for review but we shouldn't merge this until the final release is out.

@sentrivanasentrivana linked an issue Apr 8, 2024 that may be closed by this pull request
@sentrivana
sentrivana merged commit 467bde9 into sentry-sdk-2.0Apr 10, 2024
@sentrivana
sentrivana deleted the ivana/2.0/update-tooling branch April 10, 2024 07:37
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update tooling

2 participants

@sentrivana@antonpirker