Skip to content

deactivating all streaming tests #967

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

Closed
wants to merge 2 commits into from
Closed

Conversation

Kully
Copy link
Contributor

@Kully Kully commented Mar 9, 2018

@bpostlethwaite @cldougl
I'm vote to deactivate stream testing because of intermittent test failures on circle that are preventing otherwise merge-able PRs (current example for me is #964).

WDYT?

I think there are some plans to remove streaming anyways, so I don't see this as a big loss or move away from proper code testing practice.

@bpostlethwaite
Copy link
Member

We have deprecated streaming from our Cloud product but we intend to support streaming from OnPrem for the forseeable future.

Removing streaming testing could lead to simpler (and seemingly more efficient) development on this library with the risk of surprising delays during OnPrem release windows.

I don't have a good sense of the risk / reward. How coupled is the streaming functionality to the core of this library?

cc @scjody

@scjody
Copy link

scjody commented Mar 12, 2018

Streaming is still supported in On-Prem, so I don't think it's a good idea to remove the streaming tests. If we remove the tests, we'll have to do more manual testing at streambed release time (and this is generally done by one of our more senior engineers). Also if we discover failures at that time, they will be much harder to fix. The sooner you can catch a failure the easier it is to address. For example if one of us creates a PR that breaks streaming and it's caught by a CI test, the person doing the PR will know right away and they will be able to address it while the PR is fresh in their mind. If we wait to test until the next On-Prem release (up to 4 months away), the design and implementation decisions made during the PR will be long-forgotten.

How much time would it take to address the current intermittent failures?

@Kully
Copy link
Contributor Author

Kully commented Mar 12, 2018

How much time would it take to address the current intermittent failures?

I personally do not know. I think @cldougl was telling me that she was working to remove these tests, but do not know what the timing would be nor how difficult.

@cldougl
Copy link
Member

cldougl commented Mar 12, 2018

@Kully not working to remove these tests as the streaming functionality is still place for current users and on-prem users.
I'm working on updating the documentation to make this more clear: https://plot.ly/python/streaming-tutorial/#streaming-support

Most of the intermittent test failures for this repo should be improved with: #949

Do you have a screenshot of the test error that was due to the streaming server?

@Kully
Copy link
Contributor Author

Kully commented Mar 12, 2018

Also if we discover failures at that time, they will be much harder to fix.

Good point. I'm in agreement

@Kully
Copy link
Contributor Author

Kully commented Mar 12, 2018

@Kully not working to remove these tests as the streaming functionality is still place for current users and on-prem users.

My mistake. I remember now you telling me you were fixing the tests.

I can get you a screenshot/a code snippet when I see it next time.

@Kully Kully closed this Mar 13, 2018
@scjody scjody deleted the remove-streaming-tests branch March 13, 2018 19:22
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.

4 participants