-
Notifications
You must be signed in to change notification settings - Fork 124
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
Added specific pipes property constants #374
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can just get rid of all the pipes in java and add the properties in python directly.
python/zingg/pipes/pipes.py
Outdated
Pipe.__init__(self, name, "bigquery") | ||
Pipe.addProperty(self, "viewsEnabled", "true") | ||
Pipe.__init__(self, name, Format.BIGQUERY.type()) | ||
Pipe.addProperty(self, JBigQueryPipe.VIEWS_ENABLED, "true") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we add this property by default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Java app, there are no settings for BigQueryPipe,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Property is no more set here.
python/zingg/pipes/pipes.py
Outdated
@@ -6,46 +6,48 @@ | |||
|
|||
Format = jvm.zingg.client.pipe.Format | |||
FilePipe = jvm.zingg.client.pipe.FilePipe | |||
JSnowflakePipe = jvm.zingg.client.pipe.SnowflakePipe | |||
JBigQueryPipe = jvm.zingg.client.pipe.BigQueryPipe | |||
|
|||
class CsvPipe(Pipe): | |||
def __init__(self, name): | |||
Pipe.__init__(self, name, Format.CSV.type()) | |||
Pipe.addProperty(self, FilePipe.HEADER,"true") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is wrong - we can not set header default true. we have to let the user set the values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've removed it now.
fe83e27
to
426a38d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add newlines between python methods.
|
9b2beef
to
fa6e107
Compare
Merge conflicts resolved. |
No description provided.