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

Alternate approach to get location for Bigquery tables #1449

Merged
merged 7 commits into from
Dec 19, 2022

Conversation

kaxil
Copy link
Collaborator

@kaxil kaxil commented Dec 16, 2022

An alternate approach to #1416

Also closes #1460

ToDo:

  • Add more tests

The region parameter passed to Table wasn't used anywhere except internally -- so going to remove it without deprecation

@codecov
Copy link

codecov bot commented Dec 16, 2022

Codecov Report

Base: 97.32% // Head: 97.32% // No change to project coverage 👍

Coverage data is based on head (053141f) compared to base (5c69b4d).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1449   +/-   ##
=======================================
  Coverage   97.32%   97.32%           
=======================================
  Files          19       19           
  Lines         672      672           
=======================================
  Hits          654      654           
  Misses         18       18           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@kaxil kaxil force-pushed the hack-schema-location branch 3 times, most recently from 58a75cb to 95adc25 Compare December 17, 2022 00:08
@kaxil kaxil force-pushed the hack-schema-location branch from 95adc25 to 77039fe Compare December 18, 2022 15:17
@kaxil kaxil force-pushed the hack-schema-location branch from 085bd70 to bf9734f Compare December 18, 2022 15:31
@kaxil kaxil force-pushed the hack-schema-location branch from bf9734f to 4358476 Compare December 18, 2022 15:33
Copy link
Collaborator

@utkarsharma2 utkarsharma2 left a comment

Choose a reason for hiding this comment

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

A better approach will cater to other operators in the future.

python-sdk/src/astro/databases/base.py Show resolved Hide resolved
python-sdk/src/astro/databases/base.py Show resolved Hide resolved
`numpy.int` was initially deprecated and now removed in 1.24 released yday

```python
In [3]: numpy.int
<ipython-input-3-1989d03c08d8>:1: DeprecationWarning: `np.int` is a deprecated alias for the builtin `int`. To silence this warning, use `int` by itself. Doing this will not modify any behavior and is safe. When replacing `np.int`, you may wish to use e.g. `np.int64` or `np.int32` to specify the precision. If you wish to review your current use, check the release note link for additional information.
Deprecated in NumPy 1.20; for more details and guidance: https://numpy.org/devdocs/release/1.20.0-notes.html#deprecations
  numpy.int
Out[3]: int
```

This

(cherry picked from commit 745ab5f)
@kaxil kaxil marked this pull request as ready for review December 19, 2022 19:48
@kaxil kaxil dismissed dimberman’s stale review December 19, 2022 20:08

Addressed comments & Cloud IDE team is waiting for this release

@kaxil kaxil merged commit 91e4bef into main Dec 19, 2022
@kaxil kaxil deleted the hack-schema-location branch December 19, 2022 20:08
@kaxil kaxil added this to the 1.3.3 milestone Dec 19, 2022
kaxil added a commit that referenced this pull request Dec 19, 2022
An alternate approach to
#1416

Also closes #1460

The `region` parameter passed to Table wasn't used anywhere except
internally -- so going to remove it without deprecation

(cherry picked from commit 91e4bef)
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.

3 participants