custom-components / custom_updater

[DEPRECATED]📦 A component which allows you to track and update custom cards/components and python_scripts
MIT License
166 stars 66 forks source link

Support for HA 0.88.0 file structure for developers #98

Closed alandtse closed 5 years ago

alandtse commented 5 years ago

In reviewing the Developer wiki, it's not clear how we should handle the new package structure from HA 0.88.0. I wasn't sure if it's a feature issue or a documentation issue. For custom_components with multiple files, what is the appropriate way to handle multiple files?

For reference, I am addressing a feature request for alexa_media_player to leverage this component. We currently have three files, __init__.py, const.py, and media_player.py. This number will be increasing as we add more components.

Ideally, it would be a single entry with the directory name, but I haven't gotten this to work.

{
    "alexa_media": {
        "version": "1.0.1",
        "local_location": "custom_components/alexa_media",
        "remote_location": "https://raw.githubusercontent.com/alandtse/custom_components/custom_updater/alexa_media",
        "visit_repo": "https://github.com/keatontaylor/custom_components",
        "changelog": "https://github.com/keatontaylor/custom_components/wiki/Changelog"
    }
}

Is the preferred mechanism to list each file separately? That breaks your domain key mechanism and also has some redundancies in data.

{
    "alexa_media.__init__.py": {
        "version": "1.0.1",
        "local_location": "custom_components/alexa_media/__init__.py",
        "remote_location": "https://raw.githubusercontent.com/alandtse/custom_components/custom_updater/alexa_media/__init__.py",
        "visit_repo": "https://github.com/keatontaylor/custom_components",
        "changelog": "https://github.com/keatontaylor/custom_components/wiki/Changelog"
    },
    "alexa_media.const.py": {
        "version": "1.0.1",
        "local_location": "custom_components/alexa_media/const.py",
        "remote_location": "https://raw.githubusercontent.com/alandtse/custom_components/custom_updater/alexa_media/const.py",
        "visit_repo": "https://github.com/keatontaylor/custom_components",
        "changelog": "https://github.com/keatontaylor/custom_components/wiki/Changelog"
    },
    "alexa_media.media_player.py": {
        "version": "1.0.1",
        "local_location": "custom_components/alexa_media/media_player.py",
        "remote_location": "https://raw.githubusercontent.com/alandtse/custom_components/custom_updater/alexa_media/media_player.py",
        "visit_repo": "https://github.com/keatontaylor/custom_components",
        "changelog": "https://github.com/keatontaylor/custom_components/wiki/Changelog"
    }
}

Perhaps the files should be a key array? This assumes the base path is specified in local_location and remote_location.

{
    "alexa_media": {
        "version": "1.0.1",
        "local_location": "custom_components/alexa_media",
        "files": [ "__init.py", "const.py", "media_player.py" ],
        "remote_location": "https://raw.githubusercontent.com/alandtse/custom_components/custom_updater/alexa_media",
        "visit_repo": "https://github.com/keatontaylor/custom_components",
        "changelog": "https://github.com/keatontaylor/custom_components/wiki/Changelog"
    }
}

Lastly, are you aware of or have a way to do a programmatic update to the version info for the json? That's one extra place for developers to forget to update on a release. We could write an additional shell script to pull the info out of the component, but that seems like something the updater could figure out since the .py file has __version__ set.

alandtse commented 5 years ago

As a note, the data in the example json on the component page appears wrong: "local_location": "custom_components/combined/camera.py", Based off the master repo, the example is missing a /. "local_location": "/custom_components/combined/camera.py",

I also noticed the __version__ pulling does not appear to allow for an import of that information from a separate file like const.py.

        "local_version_file": "const.py",

Or even better for lazy devs, overloading to identify the py file for the version check.

        "version": "const.py",

Which in theory would check the local version and the remote version as appropriate.

ludeeus commented 5 years ago

Hi,

First: I will give you a better answer later today, I just have to finish working first. Last night I added a new key option to the json files resources (You can see example in the wiki for mail_daemon.

I have updated the local_location examples.

with the new key, this should be possible for you:

{
    "alexa_media": {
        "version": "1.0.1",
        "local_location": "/custom_components/alexa_media/__init__.py",
        "remote_location": "https://raw.githubusercontent.com/keatontaylor/custom_components/master/alexa_media/__init__.py",
        "visit_repo": "https://github.com/keatontaylor/custom_components",
        "changelog": "https://github.com/keatontaylor/custom_components/wiki/Changelog",
        "resources": [
            "https://raw.githubusercontent.com/keatontaylor/custom_components/master/alexa_media/const.py",
            "https://raw.githubusercontent.com/keatontaylor/custom_components/master/alexa_media/media_player.py"
        ]
    }
}
ludeeus commented 5 years ago

I have added some more explanation about this here https://github.com/custom-components/custom_updater/wiki/components#extra-resources

Did that cover your questions about multiple files? Regarding versioning I'm working on something, but currently not sure if I will be able to do it, so for now VERSION = 'x.x.x' or __version__ = 'x.x.x' needs to exist in the main file for the component.

ludeeus commented 5 years ago

Regarding programmatically updating the json file I'm using customjson to generate the file hosted here https://github.com/custom-components/information/blob/master/repos.json

ludeeus commented 5 years ago

https://github.com/custom-components/custom_updater/wiki/components#version-tags Importing version from other files are now supported.

@alandtse Are there any more you wanted?

alandtse commented 5 years ago

Thanks for the fast response. It looks like the local_location key just has to provide a version as it's the source of the version file to compare against the remote_location. Unless your last build changed something, I don't think it has to be the __init__.py file and it's probably beneficial to leave it open so developers can choose any appropriate version file.

I'll play around with it but for the versions-tags I was hoping that the version keys could pull it from the remote_version file instead of hard coding. This will result in one extra file pull, but I think the ease of use would make adoption easier.

While I see your script in customjson can create the json file once it's in the registry, my preference would be not to have to modify the json in our repo except to point to new files.

I'll start testing and let you know if I see any issues.

ludeeus commented 5 years ago

If you are not pointing to the __init__.py file it will not download extra resources.

Having every installation of custom_updater download and parse every component/platform for version tags will not be an option, I tried this in the past (before I settled on JSON files).

If this component is added to customsjon you will not need a JSON file in your repo. if that is of interest, I can probably add this later today.

alandtse commented 5 years ago

If you are not pointing to the __init__.py file it will not download extra resources.

I just saw that in the code. Is there any reason you want to gate the resources file on__init__.py as opposed to any valid .py file or just any file that you can read the version out of? I think this adds flexibility if you don't limit it to storing it in __init__.py

EDIT: Reason I wouldn't lock it to __init__.py is because of this warning for use case 6:

Warning Although this technique is common, beware that it will fail if sample/init.py imports packages from install_requires dependencies, which will very likely not be installed yet when setup.py is run.

Having every installation of custom_updater download and parse every component/platform for version tags will not be an option, I tried this in the past (before I settled on JSON files).

Can I ask why the limited parsing of a specific file would be problematic? I can see doing it generally without structure would be painful, but requiring a json to provide the remote file to check should limit the complexity. For example, if version is empty, try remote_location parsing. Was it just a bandwidth issue?

Importing version from other files are now supported.

For version tags, I appreciate the effort to add the feature to parse for custom_components for the version. However, I think that may not scale well. I've avoided hard codingcustom_components in the file because part of my testing is to also drop it into the home-assistant tree because that's our end goal and in that case the relative link is much more maintainable then to have a try import for custom_components or homeassistant.components.

If this component is added to customsjon you will not need a JSON file in your repo.

While I appreciate adding to customjson to the master will solve my problem, I'm thinking in terms of scalability so you don't have the maintenance burden.

Lastly, not sure if you noticed but download is misspelled as downlaod in the function name.

alandtse commented 5 years ago

One thought on version, if you're already embedded into HA as a custom_component, couldn't you query the components __version__ and require they expose it? You wouldn't need to download anything for that.

EDIT: Sorry, I'm an idiot, that doesn't solve the remote_version problem.

ludeeus commented 5 years ago

I just saw that in the code. Is there any reason you want to gate the resources file on__init__.py as opposed to any valid .py file or just any file that you can read the version out of? I think this adds flexibility if you don't limit it to storing it in __init__.py

If your component contains multiple files for HA to load an __init__.py file has to be present, this is the reasoning.

EDIT: Reason I wouldn't lock it to __init__.py is because of this warning for use case 6:

if this was a manager for python packages I would agree, but that is not the case.

Can I ask why the limited parsing of a specific file would be problematic? I can see doing it generally without structure would be painful, but requiring a json to provide the remote file to check should limit the complexity. For example, if version is empty, try remote_location parsing. Was it just a bandwidth issue?

Yes, both bandwidth and time, especially on HassOS (Rpi) installations. If I remove the requirement of remote version tag every dev will remove that and I will end up with all the performance issues.

For version tags, I appreciate the effort to add the feature to parse for custom_components for the version.

I tried various methods for this, but this is the best I could get working stable enough.

Lastly, not sure if you noticed but download is misspelled as downlaod in the function name.

I did not, I had to read that 5 times to see the difference :P

alandtse commented 5 years ago

if this was a manager for python packages I would agree, but that is not the case.

Ok fair enough. My habit has been to break it up, but you're right this is a limited use case for HA so I'll migrate back to init.py

If I remove the requirement of remote version tag every dev will remove that and I will end up with all the performance issues.

Also fair point. I think it could be mitigated and would make it easier for developers, but you're right it's the developers fault for failing to update the json. ;).

I'll work with what you recommended and I'll probably submit a PR for inclusion into the official customjson once I actually merge custom_updater into the main release. I'll close this cause I think you've addressed my main concerns.

ludeeus commented 5 years ago

I will be happy to do the work to include it into customjson, the docs for that are fairly lacking. Ping me if you want me to do it :)