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

Throw an error / refuse to load if apps.yaml has a duplicate top-level key #1899

Open
1 task done
markfickett opened this issue Dec 14, 2023 · 2 comments
Open
1 task done
Labels
enhancement New feature or request

Comments

@markfickett
Copy link

Is there an existing feature request for this?

  • I have searched the existing issues

Your feature request

A couple times I've accidentally repeated the same apps.yaml top-level key. AppDaemon picks one and ignores the other (maybe this is just YAML load behavior). This has led to confusing errors for me, such as a new app working fine but an old one no longer working somewhat mysteriously until I figure out what I did. I would rather AppDaemon treat this as an error and refuse to load the new apps.yaml file.

Example:

my_app:
  module: my_app
  class: MyApp
  argument: value_one
# ... many lines ...
my_app:
  module: my_app
  class: MyApp
  argument: a_different_value

In the above case, I would like an error, but AppDaemon loads the file with only one my_app.

PyYAML has issues open for this since 2016 so I doubt we'll see a library solution, but there are a number of workarounds mentioned like creating a customized loader.

@markfickett markfickett added the enhancement New feature or request label Dec 14, 2023
@acockburn
Copy link
Member

acockburn commented Dec 15, 2023 via email

@markfickett
Copy link
Author

Thanks for taking a look!

What do you think of the approach of customizing the yaml Loader? Here's an example from a gist discussion thread:

class UniqueKeyLoader(yaml.SafeLoader):
    def construct_mapping(self, node, deep=False):
        mapping = set()
        for key_node, value_node in node.value:
            if ':merge' in key_node.tag:
                continue 
            key = self.construct_object(key_node, deep=deep)
            if key in mapping:
                raise ValueError(f"Duplicate {key!r} key found in YAML.")
            mapping.add(key)
        return super().construct_mapping(node, deep)

# other code

yaml_dic=yaml.load(yaml_file,Loader=UniqueKeyLoader)

Poking around AppDaemon's code, would that fit into utils.read_yaml_config, as a possible replacement for Loader=yaml.SafeLoader (which gets called from app_management.py) ?

By the way I'm really enjoying AppDaemon, very easy to get started, helpful examples, and flexible enough to handle as much complexity as I've come up with so far.

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

No branches or pull requests

2 participants