Skip to content
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

bug/Permit deleting db entries with leading slashes #218

Merged
merged 6 commits into from
May 3, 2024

Conversation

blast-hardcheese
Copy link
Collaborator

Why

A second attempt at #172, considering #172 (comment).

What changed

We added a DELETE method which supports multipart/form-data with fields key=${dbkey}. Canonical cURL example follows:

curl "$REPLIT_DB_URL" -X DELETE -F key=/my/slashy/key

Test plan

import asyncio
import os
import replit.database

def test():
    url = os.getenv('REPLIT_DB_URL')
    assert url
    db = replit.database.Database(url)

    print(list(db.keys()))
    print(db.set('/this/is/a/key', '/this/is/a/value'))
    print(list(db.keys()))
    # print(db.get('/this/is/a/key'))
    del db['/this/is/a/key']
    # print(db.get('/this/is/a/key'))
    print(list(db.keys()))

async def test_async():
    url = os.getenv('REPLIT_DB_URL')
    assert url
    db = replit.database.AsyncDatabase(url)

    print(await db.keys())
    print(await db.set('/this/is/a/key', '/this/is/a/value'))
    print(await db.keys())
    # print(await db.get('/this/is/a/key'))
    print(await db.delete('/this/is/a/key'))
    # print(await db.get('/this/is/a/key'))
    print(await db.keys())

test()
asyncio.run(test_async())

Rollout

Describe any procedures or requirements needed to roll this out safely (or check the box below)

  • This is fully backward and forward compatible

@blast-hardcheese blast-hardcheese requested a review from a team as a code owner May 2, 2024 03:55
@blast-hardcheese blast-hardcheese requested review from alexhulbert and removed request for a team May 2, 2024 03:55
@@ -241,8 +231,9 @@ async def delete(self, key: str) -> None:
Raises:
KeyError: Key does not exist
"""
body, content_type = encode_multipart_formdata({"key": key})
Copy link
Contributor

Choose a reason for hiding this comment

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

Are there plans to use multipart for other requests than delete?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Loose plans, yes. We already support multipart for set, migrating that codepath as well could be beneficial.

We currently have a gap for get that could be benefitted by some out-of-band encoding, but POST for "get" semantics isn't great. Unfortunately the urlspace for get is already pretty crowded, which is why this PR is just to permit people to delete the keys they've set but can't otherwise read.

When called without a refresh function we end up getting

    AttributeError: 'AsyncDatabase' object has no attribute '_refresh_timer'

on attempted close. Fix this.
This was a good first attempt, but still prevented users from deleting the keys they'd created
@blast-hardcheese blast-hardcheese force-pushed the dstewart/bug/delete-leading-slashes branch from 66f5cb1 to dd7bfd8 Compare May 2, 2024 20:22
…t supports deleting keys with leading slashes
@blast-hardcheese blast-hardcheese force-pushed the dstewart/bug/delete-leading-slashes branch from dd7bfd8 to 6821562 Compare May 2, 2024 20:27
@blast-hardcheese blast-hardcheese merged commit 0f7ac60 into master May 3, 2024
6 checks passed
@blast-hardcheese blast-hardcheese deleted the dstewart/bug/delete-leading-slashes branch May 3, 2024 16:41
@blast-hardcheese blast-hardcheese added the bug Something isn't working label May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants