[DASH-11816]: Add fallback to build images locally if dockerhub pull fails#8
Open
rsharma-zc wants to merge 1 commit intomasterfrom
Open
[DASH-11816]: Add fallback to build images locally if dockerhub pull fails#8rsharma-zc wants to merge 1 commit intomasterfrom
rsharma-zc wants to merge 1 commit intomasterfrom
Conversation
- Modified dockerhub_pull() to detect pull failures - Automatically build failed services locally using docker-compose build - Exclude frontend services (internal-frontend, external-frontend) from docker build - Benefits all projects using microservices_cli
WalkthroughThe Changes
Sequence DiagramsequenceDiagram
participant Caller
participant dockerhub_pull
participant docker-compose
participant Fallback Build
Caller->>dockerhub_pull: Start pull operation
loop For each service
dockerhub_pull->>docker-compose: pull (with cwd, suppressed output)
alt Pull succeeds
docker-compose-->>dockerhub_pull: Success
dockerhub_pull->>dockerhub_pull: Log success
else Pull fails
docker-compose-->>dockerhub_pull: Failure
dockerhub_pull->>dockerhub_pull: Queue for build (if not frontend)
end
end
alt All pulls succeeded
dockerhub_pull-->>Caller: Return with summary log
else Failures exist
dockerhub_pull->>Fallback Build: Initiate local builds
loop For each failed service
Fallback Build->>docker-compose: build (with cwd)
docker-compose-->>Fallback Build: Build result
Fallback Build->>Fallback Build: Log build outcome
end
Fallback Build->>Fallback Build: Restore working directory
Fallback Build-->>Caller: Complete
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ms/utils.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.14.2)
ms/utils.py
177-177: Starting a process with a partial executable path
(S607)
197-197: Starting a process with a partial executable path
(S607)
200-200: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
Comment on lines
+178
to
+188
| if service not in frontend_services: | ||
| log.warning('Pull failed for [{}], will build locally'.format(service)) | ||
| failed_services.append(service) | ||
| else: | ||
| log.info('Skipping build for frontend service [{}]'.format(service)) | ||
| else: | ||
| log.info('Successfully pulled images for [{}]'.format(service)) | ||
|
|
||
| for p in procs: | ||
| p.join() | ||
| if not failed_services: | ||
| log.info('All images pulled successfully from DockerHub') | ||
| return |
There was a problem hiding this comment.
Avoid claiming success when frontend pulls fail
Because frontend services are skipped instead of added to failed_services, we end up logging “All images pulled successfully” even when their pulls actually failed. That misleads anyone relying on the summary and hides necessary follow-up work. Please track skipped frontend failures separately and report them before returning success.
- frontend_services = ['internal-frontend', 'external-frontend']
+ frontend_services = ['internal-frontend', 'external-frontend']
+ skipped_frontend_services = []
@@
- else:
- log.info('Skipping build for frontend service [{}]'.format(service))
+ else:
+ log.info('Skipping build for frontend service [{}]'.format(service))
+ skipped_frontend_services.append(service)
@@
- if not failed_services:
- log.info('All images pulled successfully from DockerHub')
- return
+ if not failed_services:
+ if skipped_frontend_services:
+ log.warning(
+ 'Skipped building frontend service(s) after pull failure: {}. Please rebuild manually.'
+ .format(', '.join(skipped_frontend_services))
+ )
+ return
+ log.info('All images pulled successfully from DockerHub')
+ returnCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In ms/utils.py around lines 178 to 188, the code currently treats frontend
services as "skipped" and does not record them as failures, which leads to
logging "All images pulled successfully" incorrectly; update the logic to record
skipped frontend pull failures in a separate list (e.g.,
skipped_frontend_failures) whenever a pull fails for a service in
frontend_services, append that service to this list instead of only logging a
warning, then change the final success check to consider both failed_services
and skipped_frontend_failures (log a summary warning/list for skipped frontend
failures before returning and only log overall success when both lists are
empty). Ensure the summary messages clearly list which services failed vs. which
frontend pulls were skipped.
Author
There was a problem hiding this comment.
@CodeRabbit these changes do not make sense. Frontend docker builds fail at the moment. So for now we are fine with these changes.
End of discussion
There was a problem hiding this comment.
@rsharma-zc, understood! I've noted that the frontend builds are currently failing and the implementation is intentionally handling them this way.
✏️ Learnings added
Learnt from: rsharma-zc
PR: ZeroCater/microservices_cli#8
File: ms/utils.py:178-188
Timestamp: 2025-10-30T13:51:41.403Z
Learning: In the microservices_cli repository (ms/utils.py), frontend docker builds (internal-frontend, external-frontend) currently fail, so the dockerhub_pull() function intentionally skips building them locally when pulls fail. The success logging is acceptable even when frontend pulls fail.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
rsharma-zc
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes DASH-11816
Changes
dockerhub_pull()to detect pull failures and automatically build images locallyHow it works
docker-compose buildfor failed services (except frontends)Benefits
Summary by CodeRabbit
Bug Fixes
Refactor