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

HACS support + latest noaa_coop version #22

Open
wants to merge 30 commits into
base: master
Choose a base branch
from

Conversation

rsnodgrass
Copy link
Contributor

@rsnodgrass rsnodgrass commented Jun 27, 2022

  • add support for HACS (plus updating README.md with installation instructions)
  • editing of README and examples
  • bug fix for self.attr missing self reference
  • error message bug fix
  • bump to latest version of noaa_coop
  • ran black, refurb, isort Python code formating/standard tools

@rsnodgrass
Copy link
Contributor Author

Checking on this merge.

@rsnodgrass
Copy link
Contributor Author

rsnodgrass commented Nov 18, 2023

@jshufro Any chance this would get merged into your official version so I don't have to maintain a fixed version for others?

@jshufro
Copy link
Owner

jshufro commented Nov 18, 2023

@jshufro Any chance this would get merged into your official version so I don't have to maintain a fixed version for others?

Main issue I see is the breaking change from renaming the path to support HACS. Many people just clone this repo under custom_components.

@DanceMore
Copy link

but with HACS, people can install it and get updates without having to manage git repos at all...

I think I subscribed to this issue because I wanted to use the plugin, but I'm already managing so much software that I actually need package managers like HACS (and docker, and others) to manage the sprawl.

@jshufro
Copy link
Owner

jshufro commented Nov 19, 2023

I'm not opposed to it, just weighing the pain against the benefits.

I think it'll make peoples' lives easier long term. I'll just need to post on the forums and update the readme and all that to make the breaking change extremely well documented.

Some people don't use HACS, and it will certainly make their lives harder- now they have to git pull and cp -R the noaa_tides folder out.

@DanceMore
Copy link

I'm an SRE by trade and one of my biased opinions is "git is not a deployment tool".

You probably use git in your packaging workflows to make "Artifacts" that endusers and sysadmins install. These are ideally some form of Package Managed Object, be it .deb, .rpm, Docker images, Pypi, Rubygem, NPM, et al, or even HACS :)

Deploying directly from git is acceptable during development; heck, it's often the only choice when you're churning code and need to test it constantly... but it's hard to turn into a robust long-term solution. As you've already seen, managing paths and subdirectories can get annoying for the needs of Developers vs Deployers. It gets even more bulky and complicated if you start adding test suites, .github/ automations, or documentation that needs a toolchain to generate. Life gets very confusing when you get bug reports from someone that isn't running an "official released version number" and no git commit-ish that you can find in your history .....

@jshufro
Copy link
Owner

jshufro commented Nov 19, 2023

Python doesn't exactly need a build pipeline.

I'd prefer to have set the repo up in a hacs-compatible way from the start, but not having done that, I don't have the luxury of blindly reorganizing things without considering what might break. As an SRE I am sure you are aware.

@jshufro
Copy link
Owner

jshufro commented Nov 19, 2023

@rsnodgrass looking at the options for manifest.json, it seems like setting filename should remove the need to change the directory structure- can you test that out?

image

https://hacs.xyz/docs/publish/start/#hacsjson

If that doesn't work, i'd be more open to setting content_in_root and pulling the .py files up into the project root directory, which would let people simply git clone git@github.com:jshufro/home_assistant_noaa_tides.git noaa_tides in the custom_components path if they don't want to use HACS

@rsnodgrass
Copy link
Contributor Author

rsnodgrass commented Nov 19, 2023

@jshufro since there is only a single sensor.py, the filename approach just might work. I don't know how it works with the manifest.json file.

Another solution is to fork this into a new repo jschufro/hass-noaa-tides, add that to HACS, and then add warnings to the existing one that users should switch to the "new" one.

Thank you! Look forward to getting automated notification of updates through HACS once this is done.

"name": "NOAA Tides and Currents",
"domains": ["sensor"],
"render_readme": true,
"filename": "sensor.py"
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
"filename": "sensor.py"
"filename": "noaa_coops/sensor.py"

This would keep the path structure of the repo intact so it's the least likely to break anyone's setup.

If it isn't tenable for whatever reason, we can go with what you have here, and I'll just warn anyone who cloned into custom_components/ that they need to reclone into a subpath called noaa_coops

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jshufro No that does not work since there are only two options for HACS path structures:

  1. custom_components/<integration_name>

  2. all files in the root of the repository AND the 'content_in_root' set to True in hacs.json

There is no other path structures supported.

However, one option might be to use a symlink. This may not work on Windows based installations, but should on any Linux or Mac HA instances. Nope, that still did not pass the HACS structure check for integrations.

HOWEVER, how about the reverse. Move noaa_tides into custom_components structure like most integrations that use HACS. Then symlink noaa_tides in root to the new path. This would allow people to install this manually still using whatever mechanism they previously did (e.g. clone, etc) AND also allow HACS to work.

@rsnodgrass
Copy link
Contributor Author

rsnodgrass commented Nov 20, 2023 via email

@rjbrown99
Copy link

Should I be able to test this in HACS via adding the following?

Repository: rsnodgrass/home_assistant_noaa_tides
Category: integration

Asking as I tried that, but it comes back with an error.

<Integration rsnodgrass/home_assistant_noaa_tides> Repository structure for master is not compliant

@rsnodgrass
Copy link
Contributor Author

Should I be able to test this in HACS via adding the following?

Repository: rsnodgrass/home_assistant_noaa_tides Category: integration

Asking as I tried that, but it comes back with an error.

<Integration rsnodgrass/home_assistant_noaa_tides> Repository structure for master is not compliant

Try now, the repo structure should now be compliant.

@rsnodgrass
Copy link
Contributor Author

@jshufro see my comment above about adding a symlink from noaa_tides to custom_components/noaa_tides. This should address your earlier concern about keeping the repo structure intact for those who have some sort of manual clone/install process.

@rjbrown99
Copy link

The updated repo does indeed work with HACS now, thank you.

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.

4 participants