-
Notifications
You must be signed in to change notification settings - Fork 123
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
KendraのS3データソース対応 #294
KendraのS3データソース対応 #294
Conversation
"modelIds": [ | ||
"anthropic.claude-v2" | ||
], | ||
"imageGenerationModelIds": [ | ||
"stability.stable-diffusion-xl-v0" | ||
], |
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.
linter の diff です
// 参考にしたページ番号がある場合は、アンカーリンクとして設定する | ||
const _excerpt_page_number = item.DocumentAttributes?.find( | ||
(attr) => attr.Key === '_excerpt_page_number' | ||
)?.Value?.LongValue; | ||
return message.includes(`[^${idx}]`) | ||
? `[^${idx}]: [${item.DocumentTitle}${_excerpt_page_number ? `(${_excerpt_page_number} ページ)` : ''}](${item.DocumentURI}${_excerpt_page_number ? `#page=${_excerpt_page_number}` : ''})` | ||
: ''; | ||
}) |
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.
PDFのページ番号も参考情報として出力するようにしました
filePrefix | ||
); | ||
window.open( | ||
`${signedUrl}${anchorLink ? `#${anchorLink}` : ''}`, |
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.
#page=ページ番号
でPDFを開くことによって、指定のページ番号を初期表示にすることができます(ブラウザやPDFリーダーによって挙動が変わるかもです)
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.
_excerpt_page_number の実装すごい良いのでぜひ入れたいのですが、他のファイルフォーマットって検証 and 考慮されていますか?例えば word など。
packages/cdk/lib/construct/rag.ts
Outdated
} | ||
); | ||
if (dataSourceBucket) { | ||
dataSourceBucket.grantReadWrite(getDocDownloadSignedUrlFunction); |
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.
これ write 権限必要なんでしたっけ?
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.
すみません、うっかりミスです!write は必要ありません!
packages/cdk/lib/construct/rag.ts
Outdated
@@ -189,5 +237,15 @@ export class Rag extends Construct { | |||
new LambdaIntegration(retrieveFunction), | |||
commonAuthorizerProps | |||
); | |||
|
|||
const docResource = ragResource.addResource('doc'); | |||
// POST: /doc/download-url |
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.
上のコメントも間違っているようですが、正しくは /rag/doc/download-url かも?
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.
上の部分も含めて修正しました。
こちらはPDFしか対応していないようです。PDF以外だと、属性自体付与されませんでした。
|
// 日本語ファイル名の場合は URI エンコードされたファイル名が URL に設定されているため、 | ||
// デコードしてから署名付き URL を取得する(二重で URI エンコードされるのを防止する) | ||
decodeURIComponent(filePrefix) |
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.
S3のURLは日本語のファイル名がURIエンコードされた状態になっており、そのURIエンコードされたファイル名を使って署名付きURLを発行するとエラーになるので、それを防止しています。
(S3 SDKが署名付きURLを発行する際に、ファイル名を自動でURIエンコードするため、二重でURIエンコードされた状態になります)
Issue #, if available:
#282 #189
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.