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

Deprecate settings.yml, and document usage #1692

Merged
merged 2 commits into from
Nov 30, 2023
Merged

Conversation

jotaen4tinypilot
Copy link
Contributor

@jotaen4tinypilot jotaen4tinypilot commented Nov 29, 2023

Resolves #1524.

The outcome of our discussion in the ticket was this:

  • The SQlite DB (tinypilot.db) should be the default place for new settings, whereas the YAML file (settings.yml) should only be used in exceptional cases, and obviously for legacy reasons.
  • We favor having a single storage solution over a dual approach, as that simplifies future decision-making, and eliminates potential migration burden between the two storage formats.
  • The SQlite DB doesn’t easily allow programmatic access right now (in contrast to the YAML file), so we created this ticket for removing this hurdle.
  • We will document the parameters in settings.yml separately.

This PR adds a comment in settings.py, marking its usage as quasi-deprecated. It also adds some historical context, references the SQlite DB as default storage, and lists sample exceptional cases.

I realized that we probably don’t need to add any documentation to the SQlite DB, since there is not really an alternative to it any longer. So by deprecating settings.yml, the SQlite DB is the only storage option anyway, and there shouldn’t be any ambiguity left. Contrary to my initial proposal, I also don’t think we should move settings.py up to the app package, as that might mistakenly promote its usage.
Review on CodeApprove

Copy link
Contributor

@jdeanwallace jdeanwallace left a comment

Choose a reason for hiding this comment

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

Automated comment from CodeApprove ➜

Approved on CodeApprove
✔️ Approved

Nice explanation.


👀 @jotaen4tinypilot it's your turn please take a look

@jotaen4tinypilot jotaen4tinypilot merged commit 3d15865 into master Nov 30, 2023
12 checks passed
@jotaen4tinypilot jotaen4tinypilot deleted the settings-docs branch November 30, 2023 14:12
Sign up for free to 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.

Define separation of configuration in ~/settings.yml and the SQlite DB
3 participants