-
Notifications
You must be signed in to change notification settings - Fork 89
Feat/worksheet nlq beta #1639
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
base: main
Are you sure you want to change the base?
Feat/worksheet nlq beta #1639
Conversation
…ksheet-nlq-beta # Conflicts: # backend/requirements.txt
dlpzx
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed and tested frontend and comments from yesterday
|
Some example views from the UI: Text To SQL: Document Analyzer: UserGuide Information in this PR: cc: @zsaltys @TejasRGitHub @anushka-singh |
|
@dlpzx - added additional tests and believe I have addressed all comments The one pending item is when testing the happy paths for both
If you retry the test in isolation / after some time it will resolve itself. Looking at the service quota limits, Claude 3.5 Sonnet has a fairly low max invoke count per account per minute of 50 (note: we are still far under that per minute threshold but I think that or similar could be causing this throttling exception - other Claude 3 models have a much higher per minute tolerance of invocations) Not sure if we should opt for a Claude 3 version for the time being to remediate this testing case and also to improve UX for a user using these features, thoughts? |
backend/dataall/modules/worksheets/aws/bedrock_prompts/process_text_template.txt
Outdated
Show resolved
Hide resolved
dlpzx
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but needs to resolve conflicts
| def __init__(self): | ||
| self._session = SessionHelper.get_session() | ||
| self._client = self._session.client('bedrock-runtime') | ||
| model_id = 'anthropic.claude-3-5-sonnet-20240620-v1:0' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am facing errors of the type: An error occurred (ValidationException) when calling the InvokeModel operation: Invocation of model ID anthropic.claude-3-5-sonnet-20240620-v1:0 with on-demand throughput isn’t supported. Retry your request with the ID or ARN of an inference profile that contains this model. I think it is related to the enforced cross-region inference
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if you have faced something similar when using this model
# Conflicts: # backend/dataall/modules/s3_datasets/services/dataset_service.py # backend/dataall/modules/worksheets/api/resolvers.py # backend/dataall/modules/worksheets/services/worksheet_service.py # deploy/stacks/lambda_api.py # frontend/src/modules/Worksheets/views/WorksheetView.js
d42b9fc to
27e2b8b
Compare
~~⚠️ merge after #1639~~ (cherry-picked the resource_thresholds feature) ### Feature or Bugfix - Feature ### Detail - Implements #1599 - see design for full explanation ### Relates - #1599 ### Security Please answer the questions below briefly where applicable, or write `N/A`. Based on [OWASP 10](https://owasp.org/Top10/en/). - Does this PR introduce or modify any input fields or queries - this includes fetching data from storage outside the application (e.g. a database, an S3 bucket)? - Is the input sanitized? - What precautions are you taking before deserializing the data you consume? - Is injection prevented by parametrizing queries? - Have you ensured no `eval` or similar functions are used? - Does this PR introduce any functionality or component that requires authorization? - How have you ensured it respects the existing AuthN/AuthZ mechanisms? - Are you logging failed auth attempts? - Are you using or adding any cryptographic features? - Do you use a standard proven implementations? - Are the used keys controlled by the customer? Where are they stored? - Are you introducing any new policies/roles/users? - Have you used the least-privilege principle? How? By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. --------- Co-authored-by: Pelin Kuran <52086912+pelinKuran@users.noreply.github.com> Co-authored-by: kalosp <kalosp@amazon.com>


Feature or Bugfix
Detail
Relates
Security
Please answer the questions below briefly where applicable, or write
N/A. Based onOWASP 10.
fetching data from storage outside the application (e.g. a database, an S3 bucket)?
evalor similar functions are used?By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.