Skip to content

[DASH-11816]: Add fallback to build images locally if dockerhub pull fails #8

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.

Sign up for GitHub

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

Open
rsharma-zc wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[DASH-11816]: Add fallback to build images locally if dockerhub pull fails #8

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 34 additions & 11 deletions ms/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,21 +162,44 @@ def top_docker_compose_services(services):

def dockerhub_pull(services):
"""
Pull DockerHub images for the passed services in parallel.
Pull DockerHub images for the passed services. If pull fails, build locally.
"""
docker_services = get_list_of_services(services)
service_list = construct_docker_compose_file(docker_services)

procs = []
for service in service_list:
process = Process(target=run_docker_compose_command, args=(['pull', service],))
process.start()
procs.append(process)
failed_services = []
frontend_services = ['internal-frontend', 'external-frontend']

for service in docker_services:
service_dir = os.path.join(BASE_DIR, service)
if not os.path.isdir(service_dir) or not os.path.isfile(os.path.join(service_dir, 'docker-compose.yml')):
continue

log.info('Pulling images for [{}]...'.format(service))
if subprocess.call(['docker-compose', 'pull'], cwd=service_dir, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) != 0:
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
Comment on lines +178 to +188
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Oct 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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')
+        return

Committable 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.

Copy link
Copy Markdown
Author

@rsharma-zc rsharma-zc Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.


remove_docker_compose_file()
log.info('Building {} service(s) locally: {}'.format(len(failed_services), ', '.join(failed_services)))
original_dir = os.getcwd()
try:
for service in failed_services:
log.info('Building Docker images for [{}]...'.format(service))
try:
os.chdir(os.path.join(BASE_DIR, service))
subprocess.check_call(['docker-compose', 'build'])
log.info('Successfully built images for [{}]'.format(service))
except subprocess.CalledProcessError as e:
log.error('Failed to build images for [{}]: {}'.format(service, e))
finally:
os.chdir(original_dir)


def git_clone_repo(remote_url, target_dir):
Expand Down