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

Add notes about the Google Assistant integration being broken #8076

Merged
merged 19 commits into from
Dec 16, 2024

Conversation

inventor96
Copy link
Contributor

No description provided.

@bewest
Copy link
Member

bewest commented Aug 16, 2023

Thanks for confirming, should we remove the code and log a deprecation warning as well?

@inventor96
Copy link
Contributor Author

We could, but I'm wondering if it might be helpful to keep it in case we can revive it again, or if someone figures out a good way to use one of Google's alternatives to Conversational Actions. As far as I'm aware, it doesn't expose any security vulnerabilities, and it doesn't add overhead to the application.

The code that supports Google Assistant specifically is just a "translator" that sits between the Google APIs and the generic virtual assistant code in Nightscout. The Alexa integration also depends on the generic part of the code, so we at least don't want to remove that part for sure.

@bewest
Copy link
Member

bewest commented Aug 16, 2023

I'm interested in otherwise comparable alternatives. To the extent we can remove any dependencies, we would like to do so, especially non-functional ones. We've spent the better part of the last year working on bringing dependencies up to date and to prepare for removing things that aren't going to help us move forward. It makes sense to me to remove all the google-assistant specific code and replace it with a warning on the console, especially if it allows us to remove a dependency, especially any additional http libraries.

@inventor96
Copy link
Contributor Author

Good to know. Thankfully there's no dependencies

@inventor96 inventor96 closed this Aug 16, 2023
@inventor96
Copy link
Contributor Author

Whoops, wrong button.

There's no dependencies that are unique to the Google Assistant integration, so we're clear there. I suppose we could remove the code, and maybe I'll stick a link in the docs to the last commit with the code. That way anyone who figures out a good way to do it again can use it as a reference.

@inventor96 inventor96 reopened this Aug 16, 2023
@inventor96
Copy link
Contributor Author

In what context do you think a warning log message will be needed? Like if they have the plugin in the enabled list?

@bewest
Copy link
Member

bewest commented Nov 29, 2024

Yes, if someone enables the plugin, what should happen?
I now face a similar issue with medtronic specific support for nighscout-connect.

@inventor96
Copy link
Contributor Author

As it stands right now, if the plugin is enabled, the app will have the routes setup for this plugin. Since nothing will be making calls to those API endpoints, nothing will be happening after that. There's no "active" part to this plugin; it's all "reactive" since nothing happens until a request is made to that specific endpoint.

@bewest bewest merged commit 7d209bc into nightscout:dev Dec 16, 2024
14 checks passed
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.

2 participants