Repository navigation
Kafka source #22
Description
The kafka IO PR #19 needs some follow ups.
- It appears to have broken the build? Or another merge removed an import. IOException symbol not found in ImportJob.
- Import job gets google application default creds and uses options.setGcpCredential? Why? I've not needed to do that before if I set them in my environment. It would also seems odd to get an error logged about it if you're using the direct or flink runner and not using GCP at all.
- ImportJobOptions should not inherit FlinkPipelineOptions and GcpOptions, as that suggests it should always use Flink and GCP. These should already be registered by default, so you can just make use of them using options.as(GcpOptions.class) if needed.
Activity
tims commentedon Dec 31, 2018
I also feel that the deserializer classes should be either inner classes or in a separate package with the feature row kafka IO class. As they are very kafka specific. But we can just move them when we get around to modularizing all the source classes.
zhilingc commentedon Dec 31, 2018
I think i accidentally removed the IOException import when I did the merge for PR #12, sorry :(
tims commentedon Dec 31, 2018
I'm fixing it, pull request incoming
tims commentedon Dec 31, 2018
@baskaranz can you take a look at this.
Also, can you please use the google-java-format plugin if you're using intellij, we standardised on that.
tims commentedon Dec 31, 2018
I think we should also write an integration test before we close this issue.
woop commentedon Jan 3, 2019
I agree. Not having an integration test is part of the challenge. We merge based on trust that all relevant testing was done. Integration tests would remove that problem.
tims commentedon Jan 3, 2019
@baskaranz can you take on making unit test for the FeatureRowKafkaIO?
baskaranz commentedon Jan 3, 2019
tims commentedon Jan 10, 2019
@baskaranz any updates on this issue?
As discussed in #13. Can you also remove the FeatureRowKey proto message? We don't actually care what the key is, as everything is in the message, so we don't need it when reading. That's a concern for the publisher.
zhilingc commentedon Jan 19, 2019
One more bug that I'd like to tag to this issue:
On the core side, the import spec validation prevents the submission of kafka jobs.
woop commentedon Jan 26, 2019
@tims @baskaranz @zhilingc, what is the current status of this issue, are we still pending the following?
baskaranz commentedon Feb 14, 2019
@woop apart from
Creation of unit tests for FeatureRowKafkaIO, the rest of the items were taken care since there's an issue running the tests using FeatureRowKafkaIO. I'm working with @pradithya on this. It works anyway, we use it to read from feature stream. We can close this issue for now and open a new issue forCreation of unit tests for FeatureRowKafkaIO.Algattik/654 fraud scenario (feast-dev#22)
Merge pull request feast-dev#22 from dmartinol/feast-rbac