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

Release @google-cloud/bigquery v2.1.1 #396

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,24 @@

[1]: https://www.npmjs.com/package/@google-cloud/bigquery?activeTab=versions

## v2.1.1

03-21-2019 11:40 PDT

### Bug Fixes
- fix: correctly encode nested custom date/time parameters ([#393](https://github.com/googleapis/nodejs-bigquery/pull/393))
- fix(job): check for `errorResult` when polling jobs ([#387](https://github.com/googleapis/nodejs-bigquery/pull/387))
Copy link
Contributor

@stephenplusplus stephenplusplus Mar 21, 2019

Choose a reason for hiding this comment

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

This is a tricky kind of fix, since the interpretation against semver isn't always a clear win for the user. This fix could be breaking change-ish for how the user has handled Jobs in the past.

Before: Job emits 'error'.
After: Job completes successfully.

We don't really know what the user does in each of those scenarios, although it seems relatively safe to fail less for the user.

So I don't know if we should bump the major for this (anyone think we should?), but at least calling it out in the docs release notes might be helpful to someone.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You make a good point and I believe @bcoe shares a similar opinion. I would be curious to hear what others think as well. While this fix is breaking-ish, it's only because it wasn't implemented correctly to begin with. I think that implies users weren't expecting the code to behave the way it did and that a patch would be appropriate.

Copy link
Contributor

Choose a reason for hiding this comment

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

Personally, I would lean towards calling this a breaking change:

  1. if you're making an effort to adhere to semver, it's important to move away from attaching too much meaning to the major digit; changing the major just means you've made a breaking API change, it doesn't need to correlate with an exciting new release of your application.
  2. @google-cloud/bigquery has enough users that I'd bet at least one of them was relying on the fact that results would populate even though an error was occurring.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bcoe all sounds reasonable to me! I've never had much of a say in our breaking change policies, so I would delegate to @JustinBeckwith on that one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FWIW I think this functionality has been broken since 2.0.4 (December) and all body errors have been ignored entirely.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's compromise and call out the change in the CHANGELOG, as:

Note: a bug was fixed that some people may have been relying on, results were still be populated in error
responses when an API returned more than the maximum configured errors. 

I think we need to better define our policies around breaking API changes, in relation to fixes, and I don't want to block this release.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is going to drive some one crazy, but I'm ok trying to make a major release (assuming no one objects) and also trying to squeeze #385 in with it.

Copy link
Contributor

Choose a reason for hiding this comment

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

SGTM. We closing this out then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yar


### Documentation
- docs(samples): adds queryParamsNamed and queryParamsPositional ([#381](https://github.com/googleapis/nodejs-bigquery/pull/381))
- refactor(samples): fix loadJSONFromGCSTruncate wrong function ([#386](https://github.com/googleapis/nodejs-bigquery/pull/386))
- refactor(samples): split query and table samples into separate files ([#384](https://github.com/googleapis/nodejs-bigquery/pull/384))

### Internal / Testing Changes
- fix(tests): update TIMESTAMP param tests ([#394](https://github.com/googleapis/nodejs-bigquery/pull/394))
- chore: publish to npm using wombat ([#390](https://github.com/googleapis/nodejs-bigquery/pull/390))
- build: use per-repo npm publish token ([#382](https://github.com/googleapis/nodejs-bigquery/pull/382))

## v2.1.0

03-12-2019 15:30 PDT
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "@google-cloud/bigquery",
"description": "Google BigQuery Client Library for Node.js",
"version": "2.1.0",
"version": "2.1.1",
"license": "Apache-2.0",
"author": "Google LLC",
"engines": {
Expand Down
2 changes: 1 addition & 1 deletion samples/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"test": "mocha --timeout 60000"
},
"dependencies": {
"@google-cloud/bigquery": "^2.1.0",
"@google-cloud/bigquery": "^2.1.1",
"@google-cloud/storage": "^2.0.0",
"yargs": "^13.0.0"
},
Expand Down