Skip to content
New issue

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

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

Already on GitHub? # to your account

Fallback to databases in inspect-data.json when no -i options are passed #1031

Closed
wants to merge 1 commit into from

Conversation

frankier
Copy link

@frankier frankier commented Oct 19, 2020

Currenlty Datasette.__init__ checks immutables against None to decide whether to fallback to inspect-data.json. This patch modifies the serve command to pass None when no -i options are passed so this fallback works correctly.

Pass empty list of immutables from serve command as None
@codecov
Copy link

codecov bot commented Oct 19, 2020

Codecov Report

Merging #1031 into main will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1031      +/-   ##
==========================================
- Coverage   84.63%   84.60%   -0.03%     
==========================================
  Files          28       28              
  Lines        3892     3892              
==========================================
- Hits         3294     3293       -1     
- Misses        598      599       +1     
Impacted Files Coverage Δ
datasette/cli.py 74.22% <ø> (ø)
datasette/views/index.py 96.49% <0.00%> (-1.76%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 568bd7b...7e7eaa4. Read the comment docs.

@simonw
Copy link
Owner

simonw commented Oct 22, 2020

I don't fully understand the bug you're fixing here. Could you provide a bit more explanation?

@frankier
Copy link
Author

The bug is that currently when there are databases passed in, but no -i flag, e.g. in configuration directory mode, inclusion in inspect-data.json does not automatically cause databases to be considered immutable, as described in the documentation.

The reason is that the -i flag is specified multiple=True, which means when it is not passed in we will get an empty list [], rather than None. So the current code decides that no databases are immutable rather than falling back to inspect-data.json -- as is presumably intended.

@frankier
Copy link
Author

Please let me know if there's anything I can do to help get this merged.

This is causing problems for me because it means when I build my Docker image my databases aren't considered immutable, which I would like them to be so that a download link is produced.

@simonw
Copy link
Owner

simonw commented Mar 29, 2021

This bug should have been fixed in #1229 - let me know if that's not the case!

Thanks

@simonw simonw closed this Mar 29, 2021
simonw added a commit that referenced this pull request Mar 29, 2021
@simonw
Copy link
Owner

simonw commented Mar 29, 2021

Sorry I didn't get to this PR sooner. I've joint-credited you in the release notes for this fix: https://docs.datasette.io/en/stable/changelog.html#v0-56

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

Successfully merging this pull request may close these issues.

2 participants