Skip to content

Conversation

@MattShirley
Copy link
Collaborator

@MattShirley MattShirley commented Nov 4, 2025

Adds image to selection criteria. Front end work for ssl-hep/ServiceX#1162

@MattShirley MattShirley linked an issue Nov 4, 2025 that may be closed by this pull request
@codecov
Copy link

codecov bot commented Nov 4, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.28%. Comparing base (54819a3) to head (c86a18e).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #680      +/-   ##
==========================================
+ Coverage   98.27%   98.28%   +0.01%     
==========================================
  Files          29       29              
  Lines        2085     2098      +13     
==========================================
+ Hits         2049     2062      +13     
  Misses         36       36              
Flag Coverage Δ
unittests 98.28% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ponyisi
Copy link
Collaborator

ponyisi commented Nov 7, 2025

Perhaps just "image"?

@ponyisi
Copy link
Collaborator

ponyisi commented Nov 18, 2025

Is there actually a backwards compatibility problem on the server side?

Could the relevant tests get added?

@MattShirley
Copy link
Collaborator Author

@ponyisi I resolved the testing issues earlier this week.

What backward compatibility issues are you thinking of? Testing locally, when I send a request to the latest develop with docker_image set, my client completes with error:

                     Delivered Files
┏━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━┓
┃ Sample         ┃ File Count ┃ Files                    ┃
┡━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━┩
│ data18_periodD │      Error │ Failed to retrieve files │
└────────────────┴────────────┴──────────────────────────┘

Total files delivered: 0

My app pod has the following logged:

Traceback (most recent call last):
  File "/home/servicex/servicex_app/resources/transformation/submit.py", line 222, in post
    ) = self.code_gen_service.generate_code_for_selection(
  File "/home/servicex/servicex_app/code_gen_adapter.py", line 83, in generate_code_for_selection
    raise ValueError(f"Failed to generate translation code: {msg}")
ValueError: Failed to generate translation code: "docker_image is not implemented. Available keys: dict_keys(['reco', 'parton', 'particle', 'max_events', 'no_systematics', 'no_filter'])" extra: {'requestId': '3f1f2dae-61d2-4a5e-af40-d6ae034dc835', 'asctime': '2025-12-03 22:10:50'}

And the codegen:

10.244.27.213 - - [03/Dec/2025:22:07:20 +0000] "POST /servicex/generated-code HTTP/1.1" 200 2472 "-" "python-requests/2.32.4"
capable python /generated/transform_single_file.py
"docker_image is not implemented. Available keys: dict_keys(['reco', 'parton', 'particle', 'max_events', 'no_systematics', 'no_filter'])"
Traceback (most recent call last):
  File "/usr/local/lib/python3.10/site-packages/servicex_codegen/post_operation.py", line 94, in post
    generated_code_result = self.code_generator.generate_code(
  File "/home/servicex/servicex/TopCP_code_generator/request_translator.py", line 70, in generate_code
    query_translate.generate_files_from_query(query, query_file_path)
  File "/home/servicex/servicex/TopCP_code_generator/query_translate.py", line 66, in generate_files_from_query
    raise KeyError(
KeyError: "docker_image is not implemented. Available keys: dict_keys(['reco', 'parton', 'particle', 'max_events', 'no_systematics', 'no_filter'])"

@kyungeonchoi
Copy link
Contributor

@MattShirley - would it be useful to check whether the user-defined image exists before send it to backend?

@MattShirley
Copy link
Collaborator Author

@kyungeonchoi I think that's out of scope for the client since it doesn't have any docker dependencies added. Even if we could do this over HTTP for Docker Hub, we'd have to come up with a solution that works for non-Docker Hub hosts (since we want to eventually support that).

@kyungeonchoi
Copy link
Contributor

@kyungeonchoi I think that's out of scope for the client since it doesn't have any docker dependencies added. Even if we could do this over HTTP for Docker Hub, we'd have to come up with a solution that works for non-Docker Hub hosts (since we want to eventually support that).

Do we have any protection for a wrong image name? Or rely on the backend error? Can you share an example value for the image field?

@MattShirley
Copy link
Collaborator Author

MattShirley commented Dec 10, 2025

@kyungeonchoi right now the client has this output:

% hatch run servicex deliver spec.yaml
Delivering spec.yaml to ServiceX cache
data18_periodD: Transform ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 0/?
                 Download ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 0/?

✓ ServiceX Delivery Complete!

                     Delivered Files
┏━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━┓
┃ Sample         ┃ File Count ┃ Files                    ┃
┡━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━┩
│ data18_periodD │      Error │ Failed to retrieve files │
└────────────────┴────────────┴──────────────────────────┘

Total files delivered: 0

And the app pod contains this in the logs: ERROR servicex servicex_app Requested transformer docker image doesn't exist: sslhep/servicex_science_image_topcp:25.2.50-v2.18.0_v0.3 extra: {'requestId': '11ecca89-e2d5-41e0-ad3b-ece9553b2dab', 'asctime': '2025-12-10 22:23:20'} corresponding to https://github.com/ssl-hep/ServiceX/blob/develop/servicex_app/servicex_app/resources/transformation/submit.py#L231 (which is already run against docker images).

@ponyisi
Copy link
Collaborator

ponyisi commented Dec 15, 2025

@MattShirley This may be something we need to fix - the client should definitely report an error. Should we not get a 500 status code?

@MattShirley
Copy link
Collaborator Author

@MattShirley This may be something we need to fix - the client should definitely report an error. Should we not get a 500 status code?

Yes, it is returning a 500 error. I can add an error message to the client output.

I am going to additionally add a mechanism for using a non-docker.io registry this week as part of this PR.

@MattShirley MattShirley changed the title add docker_image selection parameter support custom science images Jan 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support custom TopCP images

4 participants