Skip to content

Conversation

@pabloem
Copy link
Member

@pabloem pabloem commented Jan 16, 2019

The bigquery.py file has become very large. Furthermore, many of its internal classes should not be exposed to users, and will soon be used by other modules (I'm working on a BQ sink that performs file loads).

This is a refactor of a few classes ahead of the new features.

@pabloem
Copy link
Member Author

pabloem commented Jan 16, 2019

Run Portable_Python PreCommit

@pabloem
Copy link
Member Author

pabloem commented Jan 16, 2019

Run Python PostCommit

@pabloem
Copy link
Member Author

pabloem commented Jan 16, 2019

r: @chamikaramj

@pabloem pabloem changed the title Refactoring of a few BigQuery classes. [BEAM-6457] Refactoring of a few BigQuery classes. Jan 16, 2019
@pabloem
Copy link
Member Author

pabloem commented Jan 16, 2019

oops fixing the few issues

@pabloem
Copy link
Member Author

pabloem commented Jan 16, 2019

Run Python PreCommit

1 similar comment
@pabloem
Copy link
Member Author

pabloem commented Jan 17, 2019

Run Python PreCommit

@pabloem
Copy link
Member Author

pabloem commented Jan 17, 2019

streaming wordcount is having trouble. I don't know why that is. Would you mind reviewing the code while I fix that test?

@pabloem
Copy link
Member Author

pabloem commented Jan 18, 2019

Run Python PreCommit

1 similar comment
@pabloem
Copy link
Member Author

pabloem commented Jan 22, 2019

Run Python PreCommit

"Object of type '%s' is not JSON serializable" % type(obj).__name__)


class RowAsDictJsonCoder(coders.Coder):
Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately I think this (and some of the other changes below) are backwards incompatible changes to the public API. Can we leave stubs to those methods with deprecation warnings so that we don't break backwards compatibility ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I made sure to avoid moving any methods defined in __all__. Isn't __all__ a reasonable implicit include/exclude of backward compatibility guarantees?

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.

2 participants