UN-2706 [MISC] Build tools with dumb-init to enable proper SIGTERM handling in containers#1480
Conversation
kirtimanmishrazipstack
changed the title
Summary by CodeRabbit
WalkthroughBumped toolVersion fields in three tool config JSONs (classifier, structure, text_extractor), updated public tool registry entries for Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
📒 Files selected for processing (4)
tools/classifier/src/config/properties.json(1 hunks)tools/structure/src/config/properties.json(1 hunks)tools/text_extractor/src/config/properties.json(1 hunks)unstract/tool-registry/tool_registry_config/public_tools.json(2 hunks)
🔇 Additional comments (1)
tools/structure/src/config/properties.json (1)
5-5: No public registry entry found for “structure” – no update neededI searched the
unstract/tool-registry/tool_registry_config/public_tools.jsonand other JSON configs and found no"tool_uid": "structure"or similar entry. Since there’s no existing public registry record for the Structure tool, you can safely ignore the registry-update step.
chandrasekharan-zipstack
left a comment
chandrasekharan-zipstack
left a comment
There was a problem hiding this comment.
@kirtimanmishrazipstack address the comments of coderabbit, it has pointed out correctly. Also you can update backend's sample.env too to benefit OSS users. Leave it as latest if its already changed that way
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
📒 Files selected for processing (1)
unstract/tool-registry/tool_registry_config/public_tools.json(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (3)
unstract/tool-registry/tool_registry_config/public_tools.json (3)
8-8: Tool version bump for classify looks correcttoolVersion set to 0.0.66. This aligns with the stated goal of rebuilding with dumb-init.
119-119: Tool version bump for text_extractor looks correcttoolVersion set to 0.0.63, consistent with the PR narrative of rebuilding the tool images.
194-196: Resolved prior mismatch: image tag now aligned with toolVersion (text_extractor 0.0.63)image_url and image_tag updated to 0.0.63, matching properties.toolVersion. This fixes the previously flagged 0.0.62 vs 0.0.63 divergence.
Optional follow-up: ensure the images were rebuilt with dumb-init as entrypoint and pushed before merging to avoid runtime SIGTERM regressions. Use the script in my earlier comment to validate Docker Hub tags.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
backend/sample.env (1)
78-80: Reduce drift risk by avoiding duplicated image coordinates.STRUCTURE_TOOL_IMAGE_URL repeats NAME+TAG, which can drift if only one is updated. Consider:
- Using only NAME+TAG in envs and composing URL in code at runtime, or
- Adding a preflight validation to fail fast if URL and NAME:TAG don’t match.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
📒 Files selected for processing (2)
backend/sample.env(1 hunks)tools/structure/src/config/properties.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- tools/structure/src/config/properties.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (1)
backend/sample.env (1)
78-80: Version bump to tool-structure 0.0.84 looks correct.STRUCTURE_TOOL_IMAGE_URL and STRUCTURE_TOOL_IMAGE_TAG are consistently updated to 0.0.84 and align with the PR intent to pick up the new build.
kirtimanmishrazipstack
requested review from
chandrasekharan-zipstack and
johnyrahul
|
|
…ndling in containers (#1480) * build tools * build tools change * update structure tools --------- Co-authored-by: Rahul Johny <116638720+johnyrahul@users.noreply.github.com>
3 participants
What
Why
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)
Database Migrations
Env Config
Relevant Docs
Related Issues or PRs
Dependencies Versions
Notes on Testing
Screenshots
Checklist
I have read and understood the Contribution Guidelines.