Skip to content
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

Removed string helpers from consumer client #765

Conversation

simlay
Copy link
Contributor

@simlay simlay commented Feb 9, 2021

I added these string helpers in #706 because I want to be lazy. @nicholastmosher made some fair points about why they shouldn't be part of the fluvio client.

They're breaking changes so it should go out with the 0.5.0 client.

Also, this should be the last time we do some "quick fixes to the client UI", we should move to some design/review process.

Copy link
Contributor

@nicholastmosher nicholastmosher left a comment

Choose a reason for hiding this comment

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

Thanks for removing these, if it builds it LGTM

@sehz
Copy link
Contributor

sehz commented Feb 9, 2021

rebase

@simlay simlay force-pushed the remove-string-helpers-from-consumer-client branch from 80b28a9 to 217a6ef Compare February 9, 2021 20:08
@sehz
Copy link
Contributor

sehz commented Feb 9, 2021

failed in k8-test running locally:

FLUVIO_CMD=true ./target/debug/flv-test --spu 1 --produce-iteration 1000 --develop --client-log warn --server-log debug --skip-checks
Start running fluvio test runner
using k8 environment driver
Executing> /home/ubuntu/fluvio/target/debug/fluvio cluster delete
installing cluster
Executing> /home/ubuntu/fluvio/target/debug/fluvio cluster start --spu 1 --develop --rust-log debug --skip-checks
Waiting up to 60 seconds for Fluvio cluster version check...
Error: 
   0: Failed to install Fluvio on Kubernetes
   1: Timed out when waiting for SPU
panic panicked at 'fluvio cluster start should work: CommandError { command: "/home/ubuntu/fluvio/target/debug/fluvio cluster start --spu 1 --develop --rust-log debug --skip-checks", source: ExitError(1, Output { status: ExitStatus(ExitStatus(256)), stdout: "", stderr: "" }) }', tests/runner/src/environment/k8.rs:67:14
Makefile:78: recipe for target 'smoke-test-k8' failed

@simlay simlay force-pushed the remove-string-helpers-from-consumer-client branch from 217a6ef to 471dd0d Compare February 10, 2021 19:26
@simlay simlay force-pushed the remove-string-helpers-from-consumer-client branch from 471dd0d to 7a3c7b4 Compare February 11, 2021 00:16
@simlay
Copy link
Contributor Author

simlay commented Feb 11, 2021

We can revisit these changes in a later date. Strings are currently the most common use case for fluvio and this will decrease the amount of boilerplate they need to use to get started.

@simlay simlay closed this Feb 11, 2021
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