[SYNPY-1590] Implement core functionality of Submission(+Status, +Bundle) OOP model #1251

jaymedina · 2025-09-30T15:34:17Z

All tests passing:

(synapseclient) bash-3.2$ pytest tests/integration/synapseclient/models/synchronous/*submission*.py 
======================================================================= test session starts ========================================================================
platform darwin -- Python 3.9.18, pytest-8.3.3, pluggy-1.5.0
rootdir: /Users/jmedina/Documents/forks/synapsePythonClient
configfile: pytest.ini
plugins: asyncio-0.24.0, socket-0.7.0, anyio-4.6.0, mock-3.14.0
asyncio: mode=auto, default_loop_scope=session
collected 67 items                                                                                                                                                 

tests/integration/synapseclient/models/synchronous/test_submission.py .....................                                                                  [ 31%]
tests/integration/synapseclient/models/synchronous/test_submission_bundle.py ...............                                                                 [ 53%]
tests/integration/synapseclient/models/synchronous/test_submission_status.py .......................                                                         [ 88%]
tests/integration/synapseclient/models/synchronous/test_submissionview.py ........                                                                           [100%]

======================================================================= test session starts ========================================================================
platform darwin -- Python 3.9.18, pytest-8.3.3, pluggy-1.5.0
rootdir: /Users/jmedina/Documents/forks/synapsePythonClient
configfile: pytest.ini
plugins: asyncio-0.24.0, socket-0.7.0, anyio-4.6.0, mock-3.14.0
asyncio: mode=auto, default_loop_scope=session
collected 68 items                                                                                                                                                 

tests/integration/synapseclient/models/async/test_submission_async.py ......................                                                                 [ 32%]
tests/integration/synapseclient/models/async/test_submission_bundle_async.py ...............                                                                 [ 54%]
tests/integration/synapseclient/models/async/test_submission_status_async.py .......................                                                         [ 88%]

======================================================================= test session starts ========================================================================
platform darwin -- Python 3.9.18, pytest-8.3.3, pluggy-1.5.0
rootdir: /Users/jmedina/Documents/forks/synapsePythonClient
configfile: pytest.ini
plugins: asyncio-0.24.0, socket-0.7.0, anyio-4.6.0, mock-3.14.0
asyncio: mode=auto, default_loop_scope=session
collected 53 items                                                                                                                                                 

tests/unit/synapseclient/models/async/unit_test_submission_async.py ................                                                                         [ 30%]
tests/unit/synapseclient/models/async/unit_test_submission_bundle_async.py ..............                                                                    [ 56%]
tests/unit/synapseclient/models/async/unit_test_submission_status_async.py .......................                                                           [100%]

======================================================================== 53 passed in 1.61s ========================================================================

======================================================================= test session starts ========================================================================
platform darwin -- Python 3.9.18, pytest-8.3.3, pluggy-1.5.0
rootdir: /Users/jmedina/Documents/forks/synapsePythonClient
configfile: pytest.ini
plugins: asyncio-0.24.0, socket-0.7.0, anyio-4.6.0, mock-3.14.0
asyncio: mode=auto, default_loop_scope=session
collected 67 items                                                                                                                                                 

tests/unit/synapseclient/models/synchronous/unit_test_submission.py .............................                                                            [ 43%]
tests/unit/synapseclient/models/synchronous/unit_test_submission_bundle.py ...............                                                                   [ 65%]
tests/unit/synapseclient/models/synchronous/unit_test_submission_status.py .......................                                                           [100%]

synapseclient/models/submission_status.py

synapseclient/models/submission.py

…issionstatus file

…rn as evaluations design

…OR submission annotations

andrewelamb · 2025-12-02T16:05:08Z

synapseclient/models/submission.py

+            return entity_info
+        except Exception as e:
+            raise ValueError(
+                f"Unable to fetch entity information for {self.entity_id}: {e}"


Any reason not to do
raise ValueError(msg) from e ?

synapseclient/models/submission.py

andrewelamb · 2025-12-02T16:09:45Z

synapseclient/models/submission.py

+                docker_tag_response = await client.rest_get_async(
+                    f"/entity/{self.entity_id}/dockerTag"
+                )
+


Should this be an api function?

andrewelamb · 2025-12-02T16:21:35Z

synapseclient/models/submission.py

+            request_body["dockerRepositoryName"] = self.docker_repository_name
+        if self.docker_digest is not None:
+            request_body["dockerDigest"] = self.docker_digest
+


Should we have a list of optional fields and then use a for loop to iterate over them, and them to the request body if they are not none?

synapseclient/models/submission.py

andrewelamb · 2025-12-02T16:25:06Z

synapseclient/models/submission.py

+
+        Raises:
+            ValueError: If any required attributes are missing.
+        """


Should there be an example for this method?

Hmm, I don't think so, as this method won't be surfaced in the documentation. I could turn it into a hidden function instead?

synapseclient/models/submission.py

andrewelamb · 2025-12-02T17:11:04Z

synapseclient/models/submission_status.py

+
+        Returns:
+            A dictionary containing the request body for updating a submission status.
+


see #1251 (comment)

andrewelamb · 2025-12-02T17:11:45Z

synapseclient/models/submission_status.py

+            request_body["canCancel"] = self.can_cancel
+        if self.cancel_requested is not None:
+            request_body["cancelRequested"] = self.cancel_requested
+


Same suggestion as Submission about turning this into a for loop.

synapseclient/models/submission_status.py

synapseclient/annotations.py

synapseclient/models/submission.py

SageGJ · 2025-12-02T16:39:29Z

synapseclient/models/submission.py

+        except Exception as e:
+            raise ValueError(
+                f"Unable to fetch entity information for {self.entity_id}: {e}"
+            )


Should we be catching every possible exception here and assuming they're all the same cause, or would it be useful to break the catching up into synapse errors and other possible kinds? Also would a LookupError make more sense here than a ValueError?

Alternatively, we could do raise ValueError(msg) from e here. Then the user would still get a ValueError but would also see the exception that was caught as well.

tests/integration/synapseclient/models/async/test_submission_async.py

tests/integration/synapseclient/models/async/test_submission_status_async.py

SageGJ · 2025-12-02T16:52:53Z

tests/integration/synapseclient/models/async/test_submission_status_async.py

+
+        # THEN the batch update should succeed
+        assert response is not None
+        assert "batchToken" in response or response == {}  # Response format may vary


what causes the response formats to vary? should those be different parametrizations for this test?

tests/integration/synapseclient/models/synchronous/test_submission.py

SageGJ · 2025-12-02T16:57:08Z

tests/integration/synapseclient/models/synchronous/test_submission_status.py

+        assert (
+            submission_status.status is not None
+        )  # Should have some status (e.g., "RECEIVED")


same comment as similar one above

synapseclient/annotations.py

tests/integration/synapseclient/models/async/test_submission_async.py

andrewelamb · 2025-12-02T17:38:51Z

Looking good so far! I didn't see any deprecations, will you be doing that as part of this PR?

…ssion-model-functionality

synapseclient/models/submission_status.py

* always log warning * check assert called with * patch object on class instance * Revert "always log warning" This reverts commit ae63c81. * update patch target * re-add erroneously removed line

…on_id attribute)

jaymedina · 2025-12-04T19:51:15Z

Looking good so far! I didn't see any deprecations, will you be doing that as part of this PR?

@andrewelamb that's been done in PR #1252 which I'll address after addressing all the threads here.

jaymedina mentioned this pull request Oct 9, 2025

[SYNPY-1590] Implement Submission(+Status, +Bundle) OOP model #1250

Draft

15 tasks

jaymedina force-pushed the synpy-1590-submission-model-main branch from 8b50d06 to a061fb4 Compare November 5, 2025 14:46

jaymedina force-pushed the synpy-1590-submission-model-functionality branch from 62ce5fb to f0d7ecc Compare November 5, 2025 15:04

jaymedina commented Nov 12, 2025

View reviewed changes

synapseclient/models/submission_status.py Show resolved Hide resolved

jaymedina commented Nov 21, 2025

View reviewed changes

synapseclient/models/submission.py Outdated Show resolved Hide resolved

jaymedina marked this pull request as ready for review November 21, 2025 19:29

jaymedina requested a review from a team as a code owner November 21, 2025 19:29

tdd initial tests

c748132

jaymedina force-pushed the synpy-1590-submission-model-main branch from a061fb4 to c748132 Compare December 1, 2025 16:33

jaymedina added 21 commits December 1, 2025 11:35

initial intro of dataclasses

33643ae

expose api services for submission object

8a074e6

style and update docstring

c97b891

add submission and submissionstatus models

f79f683

add submission status retrieval and update methods; remove empty subm…

99ebaa1

…issionstatus file

pipe query params directly into restAPI httpx requests

33c651f

new dataclass object submission_bundle

ae05b91

move submission services functions to evaluation_services.py

f51188d

renaming imports, to_synapse_request, request body refactor

33f7a6a

patching up store method signature

4a91a2e

update docs

65eac22

new suite of tests

a5ec33a

submissionstatus rework as a mutable object

a160585

bug fix for Statuses: updated to_synapse_request to follow same patte…

660259e

…rn as evaluations design

replace != with is not for full object comparison (not just keys)

55a229c

expose the is_private arg for to_submission_status_annotations ONLY F…

3479046

…OR submission annotations

fixed submission status/submission annotations

896a3c1

add support for legacy annotations

26a02f1

remove debug prints

271181a

get_all_submission_statuses now returns a list of substat objects

c8c1041

docstring updates

0436931

andrewelamb reviewed Dec 2, 2025

View reviewed changes

SageGJ reviewed Dec 2, 2025

View reviewed changes

andrewelamb reviewed Dec 2, 2025

View reviewed changes

synapseclient/annotations.py Show resolved Hide resolved

tests/integration/synapseclient/models/async/test_submission_async.py Show resolved Hide resolved

lock mkdocstrings-python to >=2.0.0

53d9852

jaymedina force-pushed the synpy-1590-submission-model-main branch from c748132 to 31d9583 Compare December 2, 2025 19:47

jaymedina and others added 2 commits December 2, 2025 14:55

Merge branch 'synpy-1590-submission-model-main' into synpy-1590-submi…

1561141

…ssion-model-functionality

add Return description to create_submission

4d082c3

SageGJ mentioned this pull request Dec 2, 2025

[SYNPY-1714] Fix Issues with Failing Tests #1287

Merged

jaymedina commented Dec 3, 2025

View reviewed changes

synapseclient/models/submission_status.py Outdated Show resolved Hide resolved

SageGJ and others added 5 commits December 3, 2025 16:37

[SYNPY-1714] Fix Issues with Failing Tests (#1287)

2272d47

* always log warning * check assert called with * patch object on class instance * Revert "always log warning" This reverts commit ae63c81. * update patch target * re-add erroneously removed line

no need to build out Annotations object from scratch (remove evaluati…

134ad0c

…on_id attribute)

remove Dict, List imports

48468c6

fix broken tests due to removed evaluation_id attr

23a2779

style

72a4dd0

jaymedina added 13 commits December 4, 2025 15:43

import classes directly from typing. remove Dict and List.

8dd2db4

style

b75c566

add API reference links to _fetch_latest_entity

be4278a

type hint should be logging.Logger instance (generic or Synapse client)

5a70bd1

explicit import to follow the other imports

a54ade1

import order

890005e

minimize indenting by using if not:

0b0fa0c

raise ValueError(no etag for entity) sooner

96f2dd7

remove docker submission async integration test

acedf26

link Jira ticket to TODO item

75bad96

remove old cancel submission test

ea52424

remove old cancel submission test (async)

f08a6ad

assert the SubmissionStatus object has not changed

e5189d6


		Returns:
		A dictionary containing the request body for updating a submission status.

[SYNPY-1590] Implement core functionality of Submission(+Status, +Bundle) OOP model #1251

Are you sure you want to change the base?

[SYNPY-1590] Implement core functionality of Submission(+Status, +Bundle) OOP model #1251

Uh oh!

Conversation

jaymedina commented Sep 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

andrewelamb commented Dec 2, 2025

Uh oh!

Uh oh!

jaymedina commented Dec 4, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jaymedina commented Sep 30, 2025 •

edited

Loading