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

Fixed extension of config file #5803

Merged
merged 3 commits into from
Mar 23, 2023

Conversation

renaissance-design
Copy link
Contributor

Using frigate.yml as the config file for the HA addon gives a validation error, the same contents in frigate.yaml work.


1 validation error for FrigateConfig
__root__
  FrigateConfig expected dict not NoneType (type=type_error)

Using frigate.yml as the config file for the HA addon gives a validation error, the same contents in frigate.yaml work.
@netlify
Copy link

netlify bot commented Mar 22, 2023

Deploy Preview for frigate-docs ready!

Name Link
🔨 Latest commit edffe8e
🔍 Latest deploy log https://app.netlify.com/sites/frigate-docs/deploys/641b09b02c83000007e5f684
😎 Deploy Preview https://deploy-preview-5803--frigate-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@NickM-27
Copy link
Sponsor Collaborator

This is incorrect, frigate.yml has worked for many users for a long time (it was only a few versions ago that the .yaml was added)

Also the log you have shows an error in the contents itself which would have nothing to do with the file extension since frigate loads both file extensions exactly the same way

@renaissance-design
Copy link
Contributor Author

On closer inspection you're right: I created frigate.yaml on autopilot because it was in the HA config dir, then realised the docs were asking for frigate.yml, created that too and tried to populate it without deleting frigate.yaml.

So a more accurate summary of the problem would be: this application will accept two different filenames for the same config file and silently override a populated file with a 0 length one.

@NickM-27
Copy link
Sponsor Collaborator

Not 100% sure what you mean by override? Frigate does not write / edit the config file

@renaissance-design
Copy link
Contributor Author

It ignores a populated frigate.yml when an empty frigate.yaml also exists.

@NickM-27
Copy link
Sponsor Collaborator

It ignores a populated frigate.yml when an empty frigate.yaml also exists.

I mean, it has no relevance if it's empty or not. Just prefers .yaml

frigate/frigate/app.py

Lines 73 to 81 in 3f17f87

config_file = os.environ.get("CONFIG_FILE", "/config/config.yml")
# Check if we can use .yaml instead of .yml
config_file_yaml = config_file.replace(".yml", ".yaml")
if os.path.isfile(config_file_yaml):
config_file = config_file_yaml
user_config = FrigateConfig.parse_file(config_file)
self.config = user_config.runtime_config

@renaissance-design
Copy link
Contributor Author

It prefers .yaml silently then logs a config error that doesn't list the filename throwing it.

I'm sure that may seem like a non-issue to someone who maintains an app and is intimately familiar with it, but it can eat hours for someone who's completely unfamiliar with it and installing it for the first time.

@NickM-27
Copy link
Sponsor Collaborator

I'm sure that may seem like a non-issue to someone who maintains an app and is intimately familiar with it, but it can eat hours for someone who's completely unfamiliar with it and installing it for the first time.

I can see why you may have that perspective, but I spend time every day helping users in the issues with first time configuration, hardware, cameras, etc. so I have quite a bit of context for what new users experience and that is very important given the vast difference in perspective between a new user and someone who helps maintain the project.

It's important to make the docs cover potential issues without being confusing to users who may not understand what the docs are trying to say. Feel free to update this PR to communicate this behavior for the addon otherwise we can close this PR and create an issue

docs/docs/configuration/index.md Outdated Show resolved Hide resolved
Co-authored-by: Nicolas Mowen <nickmowen213@gmail.com>
@blakeblackshear blakeblackshear merged commit 1bf3b83 into blakeblackshear:dev Mar 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants