Skip to content

fix!: avoid collision with built-in functions by renaming type property to type_ #316

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.

Sign up for GitHub

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

Jump to bottom
Closed
yoshi-automation wants to merge 4 commits into from
Closed

fix!: avoid collision with built-in functions by renaming type property to type_ #316

yoshi-automation wants to merge 4 commits into from

Conversation

@yoshi-automation
Copy link
Contributor

@yoshi-automation yoshi-automation commented Oct 10, 2020 •
edited by tswast
Loading

BREAKING CHANGE: type is renamed to type_ to avoid conflict with built-in functions (introduced in googleapis/gapic-generator-python#595)

This PR was generated using Autosynth. 🌈

Synth log will be available here:
https://source.cloud.google.com/results/invocations/3d94939f-aa97-49eb-abc2-0a9a78c0de85/targets

  • To automatically regenerate this PR, check this box.

Source-Link: googleapis/synthtool@9b0da52

@yoshi-automation
changes without context
635cf80 
        autosynth cannot find the source of changes triggered by earlier changes in this
        repository, or by version upgrades to tools such as linters.
@yoshi-automation
chore(python): use BUILD_SPECIFIC_GCLOUD_PROJECT for samples
9b44eb0 
https://github.com/googleapis/python-talent/blob/ef045e8eb348db36d7a2a611e6f26b11530d273b/samples/snippets/noxfile_config.py#L27-L32

`BUILD_SPECIFIC_GCLOUD_PROJECT` is an alternate project used for sample tests that do poorly with concurrent runs on the same project.

Source-Author: Bu Sun Kim <8822365+busunkim96@users.noreply.github.com>
Source-Date: Wed Sep 30 13:06:03 2020 -0600
Source-Repo: googleapis/synthtool
Source-Sha: 9b0da5204ab90bcc36f8cd4e5689eff1a54cc3e4
Source-Link: googleapis/synthtool@9b0da52
@yoshi-automation yoshi-automation requested a review from a team October 10, 2020 12:30
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Oct 10, 2020
@tswast
Copy link
Contributor

tswast commented Oct 12, 2020

Note: there are test failures for the type -> type_ rename in schema.py logic

We have to_standard_sql on SchemaField -- googleapis/google-cloud-python#8494 but don't actually have a good use case for this function yet as far as I can tell.

Since we're making breaking changes anyway, would it make sense to remove this method until there's a concrete use case (which can be tested via system test / sample)?

tseaver
tseaver reviewed Oct 13, 2020
View reviewed changes
google/cloud/bigquery_v2/types/standard_sql.py
name = proto.Field(proto.STRING, number=1)

type = proto.Field(proto.MESSAGE, number=2, message=StandardSqlDataType,)
type_ = proto.Field(proto.MESSAGE, number=2, message=StandardSqlDataType,)
Copy link
Contributor

@tseaver tseaver Oct 13, 2020

Choose a reason for hiding this comment

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

Ugh, this is a breaking change.

Copy link
Contributor

@tswast tswast Oct 13, 2020

Choose a reason for hiding this comment

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

From talking on the Yoshi Python Chat, we have to take this :-(

Just added a commit that handles the rename in to_standard_sql

@tswast tswast changed the title [CHANGE ME] Re-generated to pick up changes from synthtool. fix!: avoid collision with built-in functions by renaming type property to type_ Oct 13, 2020
@tswast tswast mentioned this pull request Oct 13, 2020
Closed
1 task
@tswast tswast added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Oct 14, 2020
@tswast
Copy link
Contributor

tswast commented Oct 14, 2020

I'd like to wait to merge this until we make a release with the latest fixes.

After we merge this, we'll be stuck not being able to release until #319 is implemented.

This was referenced Oct 15, 2020
Closed
Closed
@product-auto-label product-auto-label bot added the api: bigquery Issues related to the googleapis/python-bigquery API. label Oct 22, 2020
@tswast tswast mentioned this pull request Nov 30, 2020
Merged
4 tasks
@tswast tswast closed this in #404 Dec 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tswast tswast tswast left review comments

+1 more reviewer

@tseaver tseaver tseaver left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

api: bigquery Issues related to the googleapis/python-bigquery API. cla: yes This human has signed the Contributor License Agreement. context: partial do not merge Indicates a pull request not ready for merge, due to either quality or timing.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@yoshi-automation @tswast @tseaver