-
-
Notifications
You must be signed in to change notification settings - Fork 32.7k
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
Add Remote calendar integration #138862
base: dev
Are you sure you want to change the base?
Add Remote calendar integration #138862
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome contribution! This is requested quite a bit so I know folks will definitely like this. 👍🏼 Very good direction, and i just took an initial pass to get the review started (didn't review the tests in detail yet)
_LOGGER = logging.getLogger(__name__) | ||
|
||
STEP_USER_DATA_SCHEMA = vol.Schema( | ||
{vol.Required(CONF_CALENDAR_NAME): str, vol.Required(CONF_URL): str} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assuming this isn't done automatically by formatting, this style key/value is easier to read for a schema:
{vol.Required(CONF_CALENDAR_NAME): str, vol.Required(CONF_URL): str} | |
{ | |
vol.Required(CONF_CALENDAR_NAME): str, | |
vol.Required(CONF_URL): str | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ruff is changing it back all the time
IcsCalendarStream.calendar_from_ics, res.text | ||
) | ||
except CalendarParseError as err: | ||
errors["base"] = "no_calendar_found" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use invalid_ics_file
consistent with the local calendar upload flow?
Also the feedback on local calendar through some bug reports is that the error is not that helpful yet (e.g. it just fails without any details, but we can fix that in the future)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, would it be okay, to import
"[%key:component::local_calendar::config::error::invalid_ics_file%]"
Or is this a dependency we don't want?
if user_input is not None: | ||
headers: dict = {} | ||
try: | ||
res = await client.get( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like aiohttp has more adoption within native home assistant code (though httpx is used of course when used by downstream client libraries). Would it be reasonable to use aiohttp here? (I don't know if there is a formal rule on this, just stating what i've observed)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think, this could also work. I just copied it from the other PR.
I'm actually more concerned about his rule:
https://developers.home-assistant.io/docs/api_lib_index?_highlight=library
Which says to create a library. But that wouldn't make much sense in my opinion, because we just download the file and handle some exceptions.
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
Thanks. Tests aren't complete anyway. I just started, with the basics. During the review there are a lot of changes anyway. So it's not really worth to test everything in the beginning. |
Breaking change
Proposed change
Add a remote calendar integration. It allows to add a read-only calendar in ics format. Like suggested here.


It's basically a copy of the local calendar and the write parts are removed. I also added a
DataUpdateCoordinator
and removed the local storage, so that the data comes from the source directly.Type of change
Additional information
Checklist
ruff format homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
.To help with the load of incoming pull requests: