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

Kafka IO fixes #23

Merged
merged 3 commits into from
Jan 2, 2019
Merged

Kafka IO fixes #23

merged 3 commits into from
Jan 2, 2019

Conversation

tims
Copy link
Contributor

@tims tims commented Dec 31, 2018

fixes for #22

@tims tims requested a review from woop December 31, 2018 05:07
@tims
Copy link
Contributor Author

tims commented Dec 31, 2018

@baskaranz

@baskaranz
Copy link
Contributor

@tims The GCP credentials is not required to be set explicitly when using Flink Runner? I know Dataflow Runner wouldn't need it explicitly but I wasn't sure If Flink Runner would work without setting it that way.

@tims
Copy link
Contributor Author

tims commented Dec 31, 2018

No it shouldn't. Anything that makes use of GCP resources like BQ or GCS should pick up credentials from the environment automatically.

For Flink in particular, we expect it's mostly likely to be used when you aren't using GCP at all. So in that case you definitely don't need to set it.

@tims
Copy link
Contributor Author

tims commented Jan 2, 2019

@baskaranz @woop can you review this when you get a chance so we can merge?

@tims tims requested a review from baskaranz January 2, 2019 06:11
Copy link
Contributor

@baskaranz baskaranz left a comment

Choose a reason for hiding this comment

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

@tims Looks good from my end

@baskaranz
Copy link
Contributor

@tims

Thought of checking on earlier comment to make myself clear

For Flink in particular, we expect it's mostly likely to be used when you aren't using GCP at all. So in that case you definitely don't need to set it.

The use case we would be using flink-runner is, when ingesting data from kafka topic(s) to BQ warehouse store and redis serving store, so flink-runner wouldn't need (or use) the GCP credentials to write to BQ in this scenario?

Copy link
Member

@woop woop left a comment

Choose a reason for hiding this comment

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

lgtm

@woop woop merged commit 1ff6035 into feast-dev:master Jan 2, 2019
@tims
Copy link
Contributor Author

tims commented Jan 3, 2019

@tims

Thought of checking on earlier comment to make myself clear

For Flink in particular, we expect it's mostly likely to be used when you aren't using GCP at all. So in that case you definitely don't need to set it.

The use case we would be using flink-runner is, when ingesting data from kafka topic(s) to BQ warehouse store and redis serving store, so flink-runner wouldn't need (or use) the GCP credentials to write to BQ in this scenario?

If you want to write to BQ as a warehouse then yes you would need gcp credentials.

I think my answer was confusing. What I meant was, you don't need to explicitly call getApplicationCredentials, because all the GCP Beam IOs already do that.

They implicitly make of the class GcpCredentialFactory .

@tims tims deleted the kafkaio branch January 3, 2019 00:25
dmartinol added a commit to dmartinol/feast that referenced this pull request Jul 1, 2024
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