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.

Sign up for GitHub

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

Jump to bottom

feat: Removed assertion logic in prompt service and other fixes #325

Merged
merged 6 commits into from
May 10, 2024
Merged

feat: Removed assertion logic in prompt service and other fixes #325

merged 6 commits into from
May 10, 2024

Conversation

chandrasekharan-zipstack
Copy link
Contributor

@chandrasekharan-zipstack chandrasekharan-zipstack commented May 9, 2024 •
edited

What

  • Removed assertion logic from prompt service
  • Dead code removed from prompt studio apps
  • Set default log level for prompt service
  • Minor log related fixes and enhancments
  • Generated a migration for a comment in tenant_account model

Why

  • Unused assertion logic can take up wasteful LLM calls
  • Flask application logs were missing for prompt service

How

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

  • Ideally no, it has only removed an assertion related logic which is currently disabled from the UI and unused

Database Migrations

  • Missing migration for a comment related change in some previous PR

Env Config

  • LOG_LEVEL for prompt service - can be INFO or DEBUG. Defaults to INFO

Relevant Docs

Related Issues or PRs

Dependencies Versions

Notes on Testing

  • Tried running some prompts in a PS project

Screenshots

image
image
image

Checklist

I have read and understood the Contribution Guidelines.

@tahierhussain tahierhussain mentioned this pull request May 9, 2024
Merged
@chandrasekharan-zipstack
Update prompt-service/src/unstract/prompt_service/main.py
a213e36 
Signed-off-by: Chandrasekharan M <117059509+chandrasekharan-zipstack@users.noreply.github.com>
@tahierhussain @Deepak-Kesavan
Removed Code Related to Assertion in the FE (#327)
76fb3b8 
Removed code related to assertion in the FE

Co-authored-by: Deepak K <89829542+Deepak-Kesavan@users.noreply.github.com>
harini-venkataraman
harini-venkataraman approved these changes May 9, 2024
View reviewed changes
Copy link
Contributor

@harini-venkataraman harini-venkataraman left a comment

Choose a reason for hiding this comment

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

LGTM

@sonarcloud SonarCloud
Copy link

sonarcloud bot commented May 10, 2024 •
edited

Quality Gate Passed Quality Gate passed

Issues
2 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 1454d90 into main May 10, 2024
4 checks passed
@nehabagdia nehabagdia deleted the feat/removed-assertion-logic-from-prompt-service branch May 10, 2024 08:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Deepak-Kesavan Deepak-Kesavan Deepak-Kesavan approved these changes

@harini-venkataraman harini-venkataraman harini-venkataraman approved these changes

@tahierhussain tahierhussain Awaiting requested review from tahierhussain

@muhammad-ali-e muhammad-ali-e Awaiting requested review from muhammad-ali-e

Assignees

@chandrasekharan-zipstack chandrasekharan-zipstack

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@chandrasekharan-zipstack @Deepak-Kesavan @harini-venkataraman @nehabagdia @tahierhussain