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

BigQuery API improvements #2663

Merged
merged 7 commits into from
May 13, 2021
Merged

Conversation

armanbilge
Copy link
Contributor

@armanbilge armanbilge commented May 12, 2021

  1. Adds a labels field to JobConfiguration and insertAllAsync. Not 100% binary compatible to 3.0.0-M1 because of case class stuff, but if important can probably be fixed?
  2. Creates a Schema singleton object to help materialize implicit TableSchema for a class T

@armanbilge armanbilge changed the title Add labels field to BigQuery JobConfiguration BigQuery API improvements May 12, 2021
Copy link
Member

@seglo seglo left a comment

Choose a reason for hiding this comment

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

LGTM. Before we merge can you enable the MimaPlugin for big query and google common? You may need to add some exceptions, but that's fine.

*/
final case class JobConfiguration(load: Option[JobConfigurationLoad]) {
final case class JobConfiguration(load: Option[JobConfigurationLoad], labels: Option[Map[String, String]]) {
Copy link
Member

Choose a reason for hiding this comment

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

We could make this constructor private to avoid breaking change, I guess? Not a big deal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if that would help? I think we need to add a constructor with the old signature. I should have marked all of these constructors private to begin with...

Copy link
Member

@seglo seglo May 12, 2021

Choose a reason for hiding this comment

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

Let's make it private and just add the MimaException. It won't be bin compat but since you added the factory method it should be code compat for the Scala DSL at least.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. While I'm at it, can I mark all the other case class constructors private?

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@seglo seglo added this to the 3.0.0 milestone May 12, 2021
@armanbilge
Copy link
Contributor Author

armanbilge commented May 12, 2021

I will open a separate PR #2665 to enable MiMa for google-common.

@seglo seglo merged commit c7ac1d7 into akka:master May 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants