-
Notifications
You must be signed in to change notification settings - Fork 4.2k
feat(ai): add endpoints to chat with ai service #8585
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
Conversation
4841595
pub merchant_id: id_type::MerchantId, | ||
pub profile_id: id_type::ProfileId, | ||
pub org_id: id_type::OrganizationId, | ||
pub query: GetDataMessage, |
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.
Would there be a situation where tenant ID might come into picture when calling the AI service?
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.
For now we are only considering merchant_id and giving result for merchant level data, if tenant id comes into picture we may have to make changes in the ai service as well to talk to the corresponding schema. Not sure, will depend on the tenant, how they want this service. This all can be as per the use case.
crates/router/src/core/chat.rs
Outdated
let request_body = chat_domain::EmbeddedAiDataRequest { | ||
query: chat_domain::GetDataMessage { | ||
message: req.message, | ||
}, | ||
org_id: user_from_token.org_id, | ||
merchant_id: user_from_token.merchant_id, | ||
profile_id: user_from_token.profile_id, | ||
}; |
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.
Should we consider logging only the identifiers (without the message or by masking the message)?
And should we consider adding metrics (indicating the number of chat requests made) as well, having identifiers as the metric attributes?
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.
logged the request with the masked message.
}, | ||
}; | ||
|
||
pub async fn get_data_from_hyperswitch_ai_workflow( |
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.
Nit: Should we consider having #[instrument]
here as well?
&auth::JWTAuth { | ||
permission: Permission::MerchantPaymentRead, | ||
}, |
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.
Any specific reason for choosing this permission? If yes, should we consider explaining it in code comments?
f92fe4d
Type of Change
Description
The PR does the following actions
Additional Changes
Motivation and Context
Closes #8584
How did you test it?
curl to test this
cargo +nightly fmt --all
cargo clippy