-
Notifications
You must be signed in to change notification settings - Fork 38
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.
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
Add endpoint_manager pause_rule methods and unit tests #203
Conversation
The TransferClient tests are getting awfully long -- we might want to try to segment those tests into a few files/modules just for ease of management/perusal. This looks good, assuming that tests pass. |
Ah, I forgot to merge params in pause_rule_list. I'm slightly disappointed in myself for this, but also proud of myself for writing tests that caught my mistake. |
795d342
to
0dcecc2
Compare
2 of the failures were a race condition I assumed would never happen in an unrelated test and a connection error. The third however is still violating my assertion that only one pause rule will be created per shared endpoint in the pause rule tests. |
Ah, I think I was somehow still looking at the old failures (maybe caused by my rebase?) since now there is only one failure. I'll test some more locally, but I think the problem is fixed now? |
Upon closer inspection, it seems the filter_host_endpoint param is either being ignored by the API, or not behaving as expected (according to the docs I should have been getting back an empty list). I removed the named parameter, but if it is behaving properly for actual host endpoint managers it can still be passed through **params. I'll add this to the list of small bugs / documentation inconsistencies with the APIs I've run into testing endpoint_manager calls. Where should I make issues to track those? |
I'd say that you should file these directly against Transfer, and that way we can figure out if they need to be fixed there (underlying bug) or just in the docs (misdocumented feature / outdated docs). If we go straight to the docs, it may get fixed to reflect the actual state of the system even if we don't really want it that way. Please do so in the language of "GET against /foo/bar", rather than the language of "SDK call TransferClient.foo(bar)". |
Resolves #197
Once this is merged I'll go back and test things that were blocked on pausing.