Add verify_ssl option for connection setup to synology_dsm by mib1185 · Pull Request #42539 · home-assistant/core

mib1185 · 2020-10-28T23:00:48Z

Proposed change

This PR make use of new features in python-synology lib v1.0.0 and add the verify_ssl option for connection setup.
Documentation is updated with home-assistant/home-assistant.io#15413
This PR resolves also an "bonus issue" from #34888
Rebase will be done, when #42523 is merged

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New integration (thank you!)
New feature (which adds functionality to an existing integration)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Additional information

This PR fixes or closes issue: fixes #
This PR is related to issue: Synology DSM discovery: flow unique_id issue? #34888
Link to documentation pull request: Add update available binary sensor and verify ssl for connection setup to synology_dsm home-assistant.io#15413

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
The code has been formatted using Black (black --fast homeassistant tests)
Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

The manifest file has all fields filled out correctly.
Updated and included derived files by running: python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt.
Updated by running python3 -m script.gen_requirements_all.
Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

No score or internal
🥈 Silver
🥇 Gold
🏆 Platinum

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

probot-home-assistant · 2020-10-28T23:11:59Z

Hey there @hacf-fr, @Quentame, mind taking a look at this pull request as its been labeled with an integration (synology_dsm) you are listed as a codeowner for? Thanks!
_{^{(message by CodeOwnersMention)}}

Quentame

LGMT, just a remark about translations.

Quentame · 2020-10-29T11:57:32Z

                    "password": "Contrasenya",
                    "port": "Port",
                    "ssl": "Utilitza un certificat SSL",
-                    "username": "Nom d'usuari"


Does all translations files are updated with this command python3 -m script.translations develop ?

If no:
You should not change translations files (except english, with the right command https://developers.home-assistant.io/docs/internationalization/core#test-translations).

Translations are handled on lokalise.com by translator contributors and update by a "translation update" commit like 026e006
See the translation documentation : https://developers.home-assistant.io/docs/translations

oh ok ... I did not know that. I have simply get the translation out of another intergration and copy&pasted them 😄
I will have a look into your mentioned approach and will update the PR properly

mib1185 · 2020-10-29T23:06:20Z

current failing/flapping tests are fixed with #42614 therefore I rebased to newest dev so that tests are re running

Quentame

Need to remove the de.json modifications.

Should be from the translation website.

mib1185 · 2020-11-02T10:27:38Z

OK, now I get it 😃 de.json is removed 👍

One short question - when will the translation for DE (and all other available languages) be added, especialy when using common string relations:

"verify_ssl": "[%key:common::config_flow::data::verify_ssl%]",

Quentame · 2020-11-02T13:20:45Z

OK, now I get it de.json is removed

One short question - when will the translation for DE (and all other available languages) be added, especialy when using common string relations:
"verify_ssl": "[%key:common::config_flow::data::verify_ssl%]",

The will be added by the "update translation" commit, see #42539 (comment)

For common string, that I am not an expert of, maybe #40578 can help 😊

Quentame · 2020-11-02T13:23:26Z

Thanks 🎉

frenck · 2020-11-02T13:36:37Z

        vol.Optional(CONF_PORT): cv.port,
-        vol.Optional(CONF_SSL, default=DEFAULT_SSL): cv.boolean,
+        vol.Optional(CONF_SSL, default=DEFAULT_USE_SSL): cv.boolean,
+        vol.Optional(CONF_VERIFY_SSL, default=DEFAULT_VERIFY_SSL): cv.boolean,


Even though this integration does support the configuration flow, we still do not accept changes to the YAML configuration.

This PR, therefore, violates ADR-0010: https://github.com/home-assistant/architecture/blob/master/adr/0010-integration-configuration.md#decision

@MartinHjelmare Should we revert?

I think we should just remove the change to the config yaml schema. The rest is ok I think.

Oh wait, we can drop yaml support 💁‍♂️

Ok, I've seen that I'm on "Watch release" on the architecture repo, that explain things 😅

Do you think it's possible to make a release when a new ADR comes out ?
So it won't spam but let followers up to date.

@Quentame Well... you have been notified of this 4 days ago as well:

See: #42539 (comment)

Indeed 😞
Was too quick on reading apparently.

To forget me, let me remove YAML support.

Proposed #42777, in draft because of a question.

bdraco · 2020-11-02T19:33:24Z

This PR broke existing config entries

2020-11-02 19:28:57 ERROR (MainThread) [homeassistant.config_entries] Error setting up entry 192.168.107.5 for synology_dsm
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/config_entries.py", line 234, in async_setup
    result = await component.async_setup_entry(hass, self)  # type: ignore
  File "/usr/src/homeassistant/homeassistant/components/synology_dsm/__init__.py", line 172, in async_setup_entry
    await api.async_setup()
  File "/usr/src/homeassistant/homeassistant/components/synology_dsm/__init__.py", line 266, in async_setup
    self._entry.data[CONF_VERIFY_SSL],
KeyError: 'verify_ssl'

…stant#42539)

probot-home-assistant Bot added the Hacktoberfest label Oct 28, 2020

homeassistant added the cla-signed label Oct 28, 2020

probot-home-assistant Bot added has-tests integration: synology_dsm Synology DSM new-feature labels Oct 28, 2020

mib1185 force-pushed the synology-dsm_verify-ssl branch from 16430fc to 0435379 Compare October 29, 2020 10:23

mib1185 marked this pull request as ready for review October 29, 2020 10:24

springstan requested changes Oct 29, 2020

View reviewed changes

Comment thread homeassistant/components/synology_dsm/__init__.py

Quentame reviewed Oct 29, 2020

View reviewed changes

mib1185 force-pushed the synology-dsm_verify-ssl branch from 0435379 to 74fc9bf Compare October 29, 2020 20:08

mib1185 requested review from Quentame and springstan October 29, 2020 20:09

mib1185 force-pushed the synology-dsm_verify-ssl branch from 74fc9bf to dfad311 Compare October 29, 2020 23:11

MartinHjelmare added the hacktoberfest-accepted label Oct 31, 2020

Quentame reviewed Oct 31, 2020

View reviewed changes

Comment thread homeassistant/components/synology_dsm/translations/de.json Outdated

Quentame mentioned this pull request Oct 31, 2020

Add update available binary sensor and verify ssl for connection setup to synology_dsm home-assistant/home-assistant.io#15413

Merged

8 tasks

mib1185 requested a review from Quentame October 31, 2020 21:43

mib1185 mentioned this pull request Nov 1, 2020

Add reboot and shutdown service to synology_dsm #42697

Merged

21 tasks

Quentame requested changes Nov 2, 2020

View reviewed changes

add verify_ssl

d6b7552

mib1185 force-pushed the synology-dsm_verify-ssl branch from dfad311 to d6b7552 Compare November 2, 2020 09:31

mib1185 requested a review from Quentame November 2, 2020 10:46

Quentame approved these changes Nov 2, 2020

View reviewed changes

Quentame merged commit 8d9a844 into home-assistant:dev Nov 2, 2020

MartinHjelmare reviewed Nov 2, 2020

View reviewed changes

Comment thread homeassistant/components/synology_dsm/const.py

frenck reviewed Nov 2, 2020

View reviewed changes

bdraco mentioned this pull request Nov 2, 2020

Existing synology_dsm config entries cannot be setup. #42764

Closed

Quentame mentioned this pull request Nov 2, 2020

Remove YAML config for synology_dsm #42777

Closed

21 tasks

mib1185 deleted the synology-dsm_verify-ssl branch January 31, 2021 19:31

Thomas55555 pushed a commit to Thomas55555/core that referenced this pull request Feb 24, 2026

Update Google Maps Travel Time API usage limits and quotas (home-assi…

3fba590

…stant#42539)

Uh oh!

Conversation

mib1185 commented Oct 28, 2020 • edited by Quentame Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed change

Type of change

Additional information

Checklist

Uh oh!

probot-home-assistant Bot commented Oct 28, 2020

Uh oh!

Uh oh!

Quentame left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mib1185 commented Oct 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Quentame left a comment

Choose a reason for hiding this comment

Uh oh!

mib1185 commented Nov 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Quentame commented Nov 2, 2020

Uh oh!

Quentame commented Nov 2, 2020

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bdraco commented Nov 2, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

mib1185 commented Oct 28, 2020 •

edited by Quentame

Loading

mib1185 commented Oct 29, 2020 •

edited

Loading

mib1185 commented Nov 2, 2020 •

edited

Loading