Skeleton dev3 #289
Skeleton dev3 #289
Conversation
Codecov ReportAttention: Patch coverage is
|
| @@ -295,6 +296,7 @@ def _build_bulk_async_endpoint( | |||
| root_ids: List, | |||
| datastack_name: str, | |||
| skeleton_version: int, | |||
| post: bool = False, | |||
There was a problem hiding this comment.
can't you just make it smart and use post if the server is above the version, there is no reason to not use post and why have the user have to specify a thing?
There was a problem hiding this comment.
i'm sorry in the review it was hard to see what this was associated with.. i see now its an internal function
There was a problem hiding this comment.
To clarify your second comment, I was certainly endeavoring to accomplish precisely that behavior, honoring the existing GET method if the server is old and doesn't offer a POST endpoint, but using POST if the server is the latest version with a snazzy POST endpoint. It's a little tedious to test it because I have to switch ltv5 back and forth between old and new versions on PrinceAllenCave and wait for them to fully deploy via CAVEdeployment to confirm the correct behavior in the two scenarios, but I can take a few minutes to do that to be sure it's working as intended.
But to be clear, that was certainly my intent.
There was a problem hiding this comment.
I suppose a properly mocked unit test can sort of test this behavior -- but I find integration tests to feel more certain. A unit test against both versions is a good idea too though.
No description provided.