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

[netatmo] Added oauth2client to feature.xml #16181

Closed
wants to merge 1 commit into from

Conversation

clinique
Copy link
Contributor

@clinique clinique commented Jan 2, 2024

I faced not started oauth2client, preventing the NetatmoHandlerFactory to initialize while working on this.

This does not seem to hurt production that much because other binding may have required the oauth2client prior to Netatmo Binding.

I stumbled on this in the morning.

@clinique clinique added the bug An unexpected problem or unintended behavior of an add-on label Jan 2, 2024
@clinique clinique self-assigned this Jan 2, 2024
@clinique clinique requested a review from lolodomo as a code owner January 2, 2024 11:21
@@ -4,6 +4,7 @@

<feature name="openhab-binding-netatmo" description="Netatmo Binding" version="${project.version}">
<feature>openhab-runtime-base</feature>
<feature>openhab-core-auth-oauth2client</feature>
Copy link
Member

@wborn wborn Jan 2, 2024

Choose a reason for hiding this comment

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

It shouldn't be necessary.

openhab-runtime-base depends on openhab-transport-http which depends on openhab-core-auth-oauth2client

If it is an issue for this add-on it's also an issue for all other add-ons using the oauth2client which would also need a fix.

@wborn
Copy link
Member

wborn commented Jan 3, 2024

The root cause is fixed with openhab/openhab-core#4012.

@wborn wborn closed this Jan 3, 2024
@jlaur
Copy link
Contributor

jlaur commented Jan 3, 2024

@wborn - I'm wondering why the Netatmo binding works for me. It seems no other bindings have this feature explicitly declared, and I see this:

openhab> feature:list | grep oauth
openhab-core-auth-oauth2client                    x 4.1.0            x          x Uninstalled x distro-4.1.0             x

I guess it shouldn't really work? Or maybe it works because I already have a refresh token and haven't needed to go through the authorization flow for some time?

Additionally, multiple bindings declare openhab-transport-http while also declaring openhab-runtime-base:

grep -R "openhab-transport-http" -B 2 -A 2

Example:

<feature name="openhab-binding-deconz" description="deCONZ Binding" version="${project.version}">
<feature>openhab-runtime-base</feature>
<feature>openhab-transport-http</feature>
<feature>openhab-transport-upnp</feature>
<bundle start-level="80">mvn:org.openhab.addons.bundles/org.openhab.binding.deconz/${project.version}</bundle>
</feature>

Is it unneeded, if openhab-runtime-base depends on openhab-transport-http?

@wborn
Copy link
Member

wborn commented Jan 3, 2024

Unfortunately I have not found any info on how <feature dependency="true"> is supposed to work in the Karaf docs at https://karaf.apache.org/manual/latest/.

After a bit of testing it looks like Karaf installs such features on demand when a feature is installed that needs it. So it probably is only an issue for users who install JARs by copying them in the /addons dir and installing such JARs using the Marketplace.

Is it unneeded, if openhab-runtime-base depends on openhab-transport-http?

The openhab-transport-http feature has been preinstalled to prevent issues since OH 2.3, see openhab/openhab-core#344.

So it should be safe to remove openhab-transport-http from add-ons that still have this in their feature.xml.
There are probably also many add-ons that use the openhab-transport-http bundles without depending on this feature in their feature.xml.

wborn added a commit to wborn/openhab-addons that referenced this pull request Jan 4, 2024
The openhab-transport-http feature dependency is redundant since OH 2.3 when it was added to the openhab-runtime-base feature dependencies.

See:

* openhab/openhab-core#344
* openhab#16181 (comment)

Signed-off-by: Wouter Born <[email protected]>
J-N-K pushed a commit that referenced this pull request Jan 4, 2024
The openhab-transport-http feature dependency is redundant since OH 2.3 when it was added to the openhab-runtime-base feature dependencies.

See:

* openhab/openhab-core#344
* #16181 (comment)

Signed-off-by: Wouter Born <[email protected]>
Cybso pushed a commit to Cybso/openhab-addons that referenced this pull request Jan 5, 2024
The openhab-transport-http feature dependency is redundant since OH 2.3 when it was added to the openhab-runtime-base feature dependencies.

See:

* openhab/openhab-core#344
* openhab#16181 (comment)

Signed-off-by: Wouter Born <[email protected]>
austvik pushed a commit to austvik/openhab-addons that referenced this pull request Mar 27, 2024
The openhab-transport-http feature dependency is redundant since OH 2.3 when it was added to the openhab-runtime-base feature dependencies.

See:

* openhab/openhab-core#344
* openhab#16181 (comment)

Signed-off-by: Wouter Born <[email protected]>
Signed-off-by: Jørgen Austvik <[email protected]>
@clinique clinique deleted the netatmo_15928_3 branch March 30, 2024 09:58
joni1993 pushed a commit to joni1993/openhab-addons that referenced this pull request Oct 15, 2024
The openhab-transport-http feature dependency is redundant since OH 2.3 when it was added to the openhab-runtime-base feature dependencies.

See:

* openhab/openhab-core#344
* openhab#16181 (comment)

Signed-off-by: Wouter Born <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An unexpected problem or unintended behavior of an add-on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants