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

Create Plugin: Update scenes app with LLM example #873

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

nmarrs
Copy link

@nmarrs nmarrs commented Apr 11, 2024

Update the create plugin template scenes app to include a example integrating the LLM plugin

NOTE: I'm not sure how to test this locally, but will work on figuring this out when I am back in next week. In meantime, with LLM plugin configured here are the demos that should be possible with these changes 🦖

basic.llm.integration.question.1.mov
basic.llm.integration.question.2.mov
advanced.llm.integration.backup.mov

Related PR: grafana/grafana-plugin-examples#273

@nmarrs nmarrs added the enhancement New feature or request label Apr 11, 2024
@nmarrs nmarrs self-assigned this Apr 11, 2024
@nmarrs nmarrs requested a review from a team as a code owner April 11, 2024 17:13
@nmarrs nmarrs requested review from oshirohugo and removed request for a team April 11, 2024 17:13
@CLAassistant
Copy link

CLAassistant commented Apr 11, 2024

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@nmarrs nmarrs requested a review from dprokop April 11, 2024 17:13
Copy link

github-actions bot commented Apr 11, 2024

Hello! 👋 This repository uses Auto for releasing packages using PR labels.

✨ This PR can be merged. It will not be considered when calculating future versions of the npm packages and will not appear in the changelogs.

@nmarrs nmarrs added the no-changelog Don't include in changelog and version calculations label Apr 11, 2024
@@ -66,7 +66,7 @@ const getDrilldownsAppScene = () => {

return new SceneAppPage({
url: prefixRoute(`${ROUTES.WithDrilldown}`) + `/room/${roomName}/temperature`,
title: `${roomName} overview`,
title: `${decodeURI(roomName)} overview`,
Copy link
Author

Choose a reason for hiding this comment

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

This change is fixing an issue that was addressed in old dead repo (see here for context)

pages: [
new SceneAppPage({
title: 'Page with LLM integration',
subTitle: 'This page showcases basic LLM integration within a scenes app',
Copy link
Member

Choose a reason for hiding this comment

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

expand the LLM abbreviation here for clarity

Copy link
Member

@academo academo left a comment

Choose a reason for hiding this comment

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

This is a nice idea but, I am not a fan of offering examples for plugins that rely in experimental apis. They will easily change, break and this example will quickly become non functional

I'd like to hear @sympatheticmoose opinion in this one

@nmarrs
Copy link
Author

nmarrs commented Apr 16, 2024

This is a nice idea but, I am not a fan of offering examples for plugins that rely in experimental apis. They will easily change, break and this example will quickly become non functional

I'd like to hear @sympatheticmoose opinion in this one

Hey @academo! Thanks for the review!

Totally understandable the concern about things breaking given the reliance of this on the grafana experimental package. For additional context, the API that this demo is using is our wrappers around OpenAIs chat completion API. Our wrapper API code has been stable since we first wrote it 8+ months ago so concern around breaking changes breaking things might not be as big of an issue :)

We will need to include some docs / instructions in the readme though on how to install and set up the LLM plugin as that is required for this demo to work. Those instructions probably could go inline if we detect the plugin isn't installed / configured correctly

@academo
Copy link
Member

academo commented Apr 17, 2024

Our wrapper API code has been stable since we first wrote it 8+ months ago so concern around breaking changes breaking things might not be as big of an issue :)

The best course of action would be to move those APIs out of the experimental package, then use the stable released APIs in this example.

Copy link
Contributor

@sympatheticmoose sympatheticmoose left a comment

Choose a reason for hiding this comment

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

Hey @nmarrs - thanks for looking at this!

The template examples we provide should be fairly barebones to allow them to act as a starting point for development, whilst neat I feel this probably overcomplicates matters and is more niche. Is there a reason we would want this in create-plugin as opposed to https://github.com/grafana/grafana-plugin-examples/tree/main/examples?

Edit: just seen grafana/grafana-plugin-examples#273 - I think this is the best route forward for now, but maybe as a separate example

@nmarrs
Copy link
Author

nmarrs commented Apr 28, 2024

Hey @nmarrs - thanks for looking at this!

The template examples we provide should be fairly barebones to allow them to act as a starting point for development, whilst neat I feel this probably overcomplicates matters and is more niche. Is there a reason we would want this in create-plugin as opposed to https://github.com/grafana/grafana-plugin-examples/tree/main/examples?

Edit: just seen grafana/grafana-plugin-examples#273 - I think this is the best route forward for now, but maybe as a separate example

Hi @sympatheticmoose! Thanks for the reply. I think the main idea behind having these examples inside create-plugin is to serve as a launching point for the community to 1) discover that LLM capabilities are possible within Grafana apps and 2) be able to use example and leverage these capabilities in their own Grafana scenes apps.

I understand your point on simplicity, maybe would only including the basic LLM integration example make sense while I could include / reference to the "more complex" example that can live over in the grafana plugin examples repo? This would still accomplish the point on discoverability but reduce the introduced complexity a fair amount.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request no-changelog Don't include in changelog and version calculations
Projects
Status: 🧑‍💻 In development
Development

Successfully merging this pull request may close these issues.

None yet

4 participants