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

pathlib.Path breaks internal schema #1365

Closed
eyeseast opened this issue Jun 7, 2021 · 1 comment · Fixed by #1370
Closed

pathlib.Path breaks internal schema #1365

eyeseast opened this issue Jun 7, 2021 · 1 comment · Fixed by #1370
Labels

Comments

@eyeseast
Copy link
Contributor

eyeseast commented Jun 7, 2021

Ran into an issue while trying to build a plugin to render GeoJSON. I'm using pytest's tmp_path fixture, which is a pathlib.Path, to get a temporary database path. I was getting a weird error involving writes, but I was doing reads. Turns out it's the internal database trying to insert a Path where it wants a string.

My test looked like this:

@pytest.mark.asyncio
async def test_render_feature_collection(tmp_path):
    database = tmp_path / "test.db"
    datasette = Datasette([database])

    # this will break with a path
    await datasette.refresh_schemas()

    # build a url
    url = datasette.urls.table(database.stem, TABLE_NAME, format="geojson")

    response = await datasette.client.get(url)
    fc = response.json()

    assert 200 == response.status_code

I only ran into this while running tests, because passing in database paths from the CLI uses strings, but it's a weird error and probably something other people have run into.

The fix is easy enough: Convert the path to a string and everything works. So this:

@pytest.mark.asyncio
async def test_render_feature_collection(tmp_path):
    database = tmp_path / "test.db"
    datasette = Datasette([str(database)])

    # this is fine now
    await datasette.refresh_schemas()

This could (probably, haven't tested) be fixed here by calling str(db.path) or by doing that conversion earlier.

@simonw
Copy link
Owner

simonw commented Jun 7, 2021

I've hit this one too. I agree, fixing this in Datasette itself is better than fixing it in the tests across multiple other projects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants