GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-100414: Skip test_dbm_sqlite3 if sqlite3 is unavailable by erlend-aasland · Pull Request #115449 · python/cpython

Conversation

@erlend-aasland

@erlend-aasland erlend-aasland commented Feb 14, 2024 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Alternative to #115448

  • Issue: Add sqlite3 as another possible backing store for the dbm module #100414

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@erlend-aasland
pythongh-100414: Skip test_dbm_sqlite3 if sqlite3 is unavailable
df23c87
@erlend-aasland
erlend-aasland requested a review from berkerpeksag as a code owner February 14, 2024 11:45
@erlend-aasland erlend-aasland added the skip news label Feb 14, 2024
@bedevere-app bedevere-app Bot added the awaiting core review label Feb 14, 2024
@bedevere-app bedevere-app Bot mentioned this pull request Feb 14, 2024
Add sqlite3 as another possible backing store for the dbm module #100414
Closed
@bedevere-app bedevere-app Bot added the tests Tests in the Lib/test dir label Feb 14, 2024
@erlend-aasland

This comment was marked as outdated.

Sign in to view
@bedevere-bot

This comment was marked as outdated.

Sign in to view
@erlend-aasland

This comment was marked as outdated.

Sign in to view
@bedevere-bot

This comment was marked as outdated.

Sign in to view
@erlend-aasland
erlend-aasland requested a review from AlexWaygood February 14, 2024 11:55
@AlexWaygood AlexWaygood mentioned this pull request Feb 14, 2024
gh-100414: Skip test_dbm_sqlite3 if sqlite3 is unavailable #115448
Closed
@erlend-aasland

erlend-aasland commented Feb 14, 2024

Copy link
Copy Markdown
Contributor Author

I think I prefer this fix.
AlexWaygood reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
AlexWaygood
AlexWaygood approved these changes Feb 14, 2024
View reviewed changes

@AlexWaygood AlexWaygood left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

See my comments in #115448 (comment). This feels slightly more fragile to me, in that whether or not the test passes depends on the order the imports are in, which isn't necessarily obvious to the reader of the test. But either is probably fine; no strong preference :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels Feb 14, 2024
@AlexWaygood

AlexWaygood commented Feb 14, 2024

Copy link
Copy Markdown
Member

This feels slightly more fragile to me, in that whether or not the test passes depends on the order the imports are in, which isn't necessarily obvious to the reader of the test.

You could maybe add a comment noting that the imports need to be in precisely that order
erlend-aasland reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@erlend-aasland

erlend-aasland commented Feb 14, 2024

Copy link
Copy Markdown
Contributor Author

You could maybe add a comment noting that the imports need to be in precisely that order

Yes, that seems fair to the future reader.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
erlend-aasland
erlend-aasland commented Feb 14, 2024
View reviewed changes
Comment thread Lib/test/test_dbm_sqlite3.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@erlend-aasland @AlexWaygood
Update Lib/test/test_dbm_sqlite3.py
767c8b0 
Co-authored-by: Alex Waygood <[email protected]>
@erlend-aasland
erlend-aasland enabled auto-merge (squash) February 14, 2024 12:28
@erlend-aasland

erlend-aasland commented Feb 14, 2024

Copy link
Copy Markdown
Contributor Author

Thanks for the review(s), Alex!
AlexWaygood reacted with rocket emoji
All reactions
  • 🚀 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@AlexWaygood
slight tweak
d9a099c
@AlexWaygood

AlexWaygood commented Feb 14, 2024

Copy link
Copy Markdown
Member

No worries!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
serhiy-storchaka
serhiy-storchaka approved these changes Feb 14, 2024
View reviewed changes
@erlend-aasland erlend-aasland mentioned this pull request Feb 14, 2024
gh-100414: Make dbm.sqlite3 the preferred dbm backend #115447
Merged
@encukou

encukou commented Feb 14, 2024

Copy link
Copy Markdown
Member

!buildbot AMD64 RHEL7
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot

bedevere-bot commented Feb 14, 2024

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @encukou for commit d9a099c 🤖

The command will test the builders whose names match following regular expression: AMD64 RHEL7

The builders matched are:

  • AMD64 RHEL7 LTO PR
  • AMD64 RHEL7 PR
  • AMD64 RHEL7 Refleaks PR
  • AMD64 RHEL7 LTO + PGO PR
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@erlend-aasland
erlend-aasland merged commit 029ec91 into python:main Feb 14, 2024
@erlend-aasland
erlend-aasland deleted the dbm/alt-fix-sqlite3-backend branch February 14, 2024 13:16
@bedevere-app bedevere-app Bot removed the awaiting merge label Feb 14, 2024
@erlend-aasland erlend-aasland mentioned this pull request Feb 14, 2024
gh-100414: Add SQLite backend to dbm #114481
Merged
fsc-eriker pushed a commit to fsc-eriker/cpython that referenced this pull request Feb 14, 2024
@erlend-aasland @AlexWaygood @fsc-eriker
pythongh-100414: Skip test_dbm_sqlite3 if sqlite3 is unavailable (pyt…
642d5f6 
…hon#115449)

Co-authored-by: Alex Waygood <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AlexWaygood AlexWaygood AlexWaygood approved these changes
@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes
@berkerpeksag berkerpeksag Awaiting requested review from berkerpeksag

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news tests Tests in the Lib/test dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@erlend-aasland @bedevere-bot @AlexWaygood @encukou @serhiy-storchaka
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.