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

[FEAT] Tool Export Validation #299

Merged
merged 7 commits into from
May 3, 2024
Merged

[FEAT] Tool Export Validation #299

merged 7 commits into from
May 3, 2024

Conversation

harini-venkataraman
Copy link
Contributor

@harini-venkataraman harini-venkataraman commented Apr 29, 2024

What

This PR enables support for exporting only valid tools that have prompts and that are executed.
...

Why

This prevents export of dummy tools developed using Prompt Studio.
...

How

Adding validations to see if the Output Manager has a valid field against the prompt.
...

Can this PR break any existing features. If yes please list of possible items. If no please explain why. (PS: Admins do not merge the PR without this section filled)

  1. Export validation is an additional filtering check and does not break features.
  2. Export functional logic has these changes made, but will prevent export only if there is not valid prompt.

...

Database Migrations

Not applicable.

...

Env Config

Not applicable.
...

Notes on Testing

  1. Test by exporting a properly executed tool.
  2. Create a prompt with no text prompt entered, try exporting.
  3. Create a prompt and without executing it, try exporting.

...

Screenshots

Screenshot from 2024-04-29 16-13-53
GT

Screenshot from 2024-04-29 15-59-04

image

Checklist

I have read and understood the Contribution Guidelines.

@harini-venkataraman harini-venkataraman changed the base branch from main to tool-export-validation April 29, 2024 12:50
@harini-venkataraman harini-venkataraman changed the base branch from tool-export-validation to main April 29, 2024 12:51
@harini-venkataraman harini-venkataraman changed the title Tool Export Validation [FEAT] Tool Export Validation Apr 29, 2024
@harini-venkataraman harini-venkataraman marked this pull request as ready for review April 29, 2024 12:55
Copy link
Contributor

@chandrasekharan-zipstack chandrasekharan-zipstack left a comment

Choose a reason for hiding this comment

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

Note that failing fast is also an option in case of a project with lot of prompts - we needn't wait for a while and throw the error. We'd unnecessarily build this JSON or make DB queries even after an invalid prompt is noted

@harini-venkataraman
Copy link
Contributor Author

Note that failing fast is also an option in case of a project with lot of prompts - we needn't wait for a while and throw the error. We'd unnecessarily build this JSON or make DB queries even after an invalid prompt is noted

@chandrasekharan-zipstack Yes, this makes sense. But in cases where one or more invalid prompts is stored in a single project, error surfacing each time export is clicked might not be a good user experience.
One thing we can do is continue the iteration if any invalid prompt is encountered. This way DB communications will be avoided. Let me validate this and see if this works.

harini-venkataraman and others added 2 commits May 2, 2024 14:22
Co-authored-by: Chandrasekharan M <117059509+chandrasekharan-zipstack@users.noreply.github.com>
Signed-off-by: harini-venkataraman <115449948+harini-venkataraman@users.noreply.github.com>
Copy link
Contributor

@chandrasekharan-zipstack chandrasekharan-zipstack left a comment

Choose a reason for hiding this comment

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

LGTM - left some NIT comments and error message enhancements which can be taken up for some easy wins on UX

harini-venkataraman and others added 3 commits May 2, 2024 17:09
Co-authored-by: Chandrasekharan M <117059509+chandrasekharan-zipstack@users.noreply.github.com>
Signed-off-by: harini-venkataraman <115449948+harini-venkataraman@users.noreply.github.com>
@harini-venkataraman harini-venkataraman self-assigned this May 2, 2024
Copy link

sonarcloud bot commented May 3, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@nehabagdia nehabagdia merged commit e28fd03 into main May 3, 2024
4 checks passed
@nehabagdia nehabagdia deleted the feat/export-validator branch May 3, 2024 10:09
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.

None yet

3 participants