Skip to content

[SYNPY-1783] Implemented grid synchronization #1368

Open
linglp wants to merge 14 commits intodevelopfrom
SYNPY-1783
Open

[SYNPY-1783] Implemented grid synchronization #1368
linglp wants to merge 14 commits intodevelopfrom
SYNPY-1783

Conversation

@linglp
Copy link
Copy Markdown
Contributor

@linglp linglp commented Apr 24, 2026

Problem:

grid can't be sync using the python client

Solution:

grid created by file view can be sync

Test

See validation instructions: https://sagebionetworks.jira.com/browse/SYNPY-1783?focusedCommentId=322977

@linglp linglp marked this pull request as ready for review May 6, 2026 16:03
@linglp linglp requested a review from a team as a code owner May 6, 2026 16:03
Copilot AI review requested due to automatic review settings May 6, 2026 16:03
@linglp linglp marked this pull request as draft May 6, 2026 16:10
@linglp linglp marked this pull request as ready for review May 6, 2026 16:16
Comment thread tests/integration/synapseclient/models/async/test_curation_async.py Outdated
Copy link
Copy Markdown
Member

@BryanFauble BryanFauble left a comment

Choose a reason for hiding this comment

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

Logic on this looks good to me, check in on tests/integration/synapseclient/models/async/test_curation_async.py and the removal of some code there.

Comment thread synapseclient/models/curation.py
Comment thread tests/integration/synapseclient/models/async/test_grid_async.py Outdated
Comment thread tests/integration/synapseclient/models/async/test_grid_async.py
Comment thread tests/integration/synapseclient/models/async/test_grid_async.py Outdated
"""

session_id: str = ""
"""The ID of the grid session to synchronize."""
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This seems like it should be required, why have a default value? Should we call this grid_session_id to be consistent with the endpoint?

Comment thread synapseclient/models/curation.py Outdated
@andrewelamb
Copy link
Copy Markdown
Contributor

@linglp Looks good overall, I found a few things to fix.

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.

3 participants