Ignore stale directories

[amelchio]

This is to ignore stale directories that can appear when an integration is removed (because __pycache__ is not cleaned up).

$ script/gen_requirements_all.py
******* ERROR
Errors while importing:  The manifest for component pollen is invalid.
Make sure you import 3rd party libraries inside methods.

[andrewsayre]

I wonder if this is too lenient now... meaning if users forget to add init, we skip the folder silently, which may lead difficulty trouble shooting. What about printing a warning that the folder/integration is being skipped?

[amelchio]

I thought that if you forget __init__.py, your component will not work at all?

[andrewsayre]

Correct -- and this has been a frequent miss for people... it will save in troubleshooting (why isn't my component working?) if we print a warning here.

[amelchio]

Hmm, it seems odd to me to add that diagnostic at this place. Shouldn't it rather be during component loading? Also, I would prefer those stale directories to not cause any noise.

I am not strongly opposed though, so let's wait for a tiebreaker opinion :)

[balloob]

We actually don't check for __init__.py, but probably should, because:

manifest.json doesn't require it, however:

• You cannot set up a component, which requires __init__.py
• You cannot set up a platform without a component

So in that case, you will need an __init__.py file.

If you don't have a manifest.json, you are required to have an __init__.py file, because it will fall back to using loader._load_file, which ignores namespace folders (folders without __init__.py).

So I think that this is a good change. However, I would suggest one change: that we print a warning message when we ignore an integration for not having __init__.py. We don't want to print a warning message when it is a file or it is the pycache dir. Have a look at #23501 for an example.

[amelchio]

I added the warning but seeing it live, I like it even less. I think it will become a FAQ.

[balloob]

Maybe we can add: If this is your development environment, you can safely delete this folder.

[amelchio]

I added that wording but I still think it would be better to just remove the warning. I don't see that this is the proper place to suggest a cleanup, just because we happen to be scanning the directories.

[balloob]

I think a warning is in place because we are skipping a folder, and the user might be adding an integration and might wonder why it is not picked up.

[balloob]

Merge if you agree with that this is the right direction.

[amelchio]

Well I don't agree, that is the use case I discussed with @andrewsayre above.

But I am not going to insist so I will merge when the tests pass.