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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Rename pybigquery to sqlalchemy-bigquery #198

Merged
merged 36 commits into from Aug 10, 2021

Conversation

@jimfulton
Copy link
Contributor

@jimfulton jimfulton commented Jul 2, 2021

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #197 馃

@tswast
Copy link
Collaborator

@tswast tswast commented Jul 2, 2021

Could we add a warning to setup.py and the README and make a release of pybigquery first? I'd like to make sure folk who look for the pybigquery package are able to find sqlalchemy-bigquery.

I'm thinking something similar to what we're doing here: https://pypi.org/project/google-cloud/

@jimfulton
Copy link
Contributor Author

@jimfulton jimfulton commented Jul 2, 2021

Could we add a warning to setup.py and the README and make a release of pybigquery first? I'd like to make sure folk who look for the pybigquery package are able to find sqlalchemy-bigquery.

I'm thinking something similar to what we're doing here: https://pypi.org/project/google-cloud/

In this case, we'd be telling people to install a package that doesn't exist, wouldn't we?

Note that pybigquery will continue to work indefinitely!

Daft plan, modified based on your comment:

  • Release an alpha of sqlalchemy-bigquery

  • Release an alpha of pybigquery that is a facade/shim for sqlalchemy-bigquery
    Non-alpha pybigquery works as expected.

    Prior to these releases, test with non-pypi installs.

  • Decide when to go to non-alpha. For sake of discussion, say, July 42. ;)

  • Make a pybigquery release that gives folks a heads up of the swap on July 42.

  • On July 42, release non-alpha sqlalchemy-bigquery and non-alpha pybigquery shim

    pybigquery shim still works but issues a deprecation warning.

    Old installs of pybigquery or installes of pinned versions still work as expected.

Thoughts @tswast ?

@jimfulton jimfulton requested a review from tswast Jul 2, 2021
@tswast
Copy link
Collaborator

@tswast tswast commented Jul 13, 2021

In this case, we'd be telling people to install a package that doesn't exist, wouldn't we?

True. Maybe we try to use some of the LTS client automation and keep around a pybigquery branch for backports? Or maybe we just manually publish a release of pybigquery once sqlalchemy-bigquery is available?

@jimfulton
Copy link
Contributor Author

@jimfulton jimfulton commented Jul 13, 2021

.repo-metadata.json Outdated Show resolved Hide resolved
Co-authored-by: Tim Swast <swast@google.com>
@jimfulton
Copy link
Contributor Author

@jimfulton jimfulton commented Jul 16, 2021

Note that we have to sort out the new name, since the new name was taken.

@cpdean
Copy link

@cpdean cpdean commented Aug 1, 2021

Note that we have to sort out the new name, since the new name was taken.

Hey @jimfulton and @tswast , I have added Jim as an owner on https://pypi.org/project/sqlalchemy_bigquery/ , and added a redirect notice on my old project at https://github.com/cpdean/sqlalchemy-bigquery .

Let me know if there's anything else I can do to help.

@jimfulton
Copy link
Contributor Author

@jimfulton jimfulton commented Aug 2, 2021

Note that we have to sort out the new name, since the new name was taken.

Hey @jimfulton and @tswast , I have added Jim as an owner on https://pypi.org/project/sqlalchemy_bigquery/ , and added a redirect notice on my old project at https://github.com/cpdean/sqlalchemy-bigquery .

Let me know if there's anything else I can do to help.

@cpdean Thank you very much!

@jimfulton
Copy link
Contributor Author

@jimfulton jimfulton commented Aug 5, 2021

@tswast are you good with this?

Copy link
Collaborator

@tswast tswast left a comment

Looking good! A couple minor comments. I think we can merge this sooner rather than later.

"release_level": "beta",
"language": "python",
"library_type": "INTEGRATION",
"repo": "googleapis/python-bigquery-sqlalchemy",
"distribution_name": "pybigquery",
"distribution_name": "sqlalchemy-bigquery"
Copy link
Collaborator

@tswast tswast Aug 5, 2021

Need extra comma now that we added api_id

Suggested change
"distribution_name": "sqlalchemy-bigquery"
"distribution_name": "sqlalchemy-bigquery",

Copy link
Contributor Author

@jimfulton jimfulton Aug 5, 2021

Fixed

except ImportError:
pass
else: # pragma: NO COVER
if not hasattr(pybigquery, "__version__"):
Copy link
Collaborator

@tswast tswast Aug 5, 2021

I'm curious what the not hasattr is doing. Could you add a comment about this? If __version__ is present it's less likely to conflict?

Copy link
Contributor Author

@jimfulton jimfulton Aug 5, 2021

That's a holdover from the shim, I think. The shim would have that attribute. I'll get rid of it.

Copy link
Contributor Author

@jimfulton jimfulton Aug 5, 2021

fixed

@jimfulton
Copy link
Contributor Author

@jimfulton jimfulton commented Aug 5, 2021

Looking good! A couple minor comments. I think we can merge this sooner rather than later.

I also just removed the api module, which we'd discussed, but that I just remembered. :)

sqlalchemy_bigquery/__init__.py Show resolved Hide resolved
sqlalchemy_bigquery/__init__.py Outdated Show resolved Hide resolved
@jimfulton jimfulton marked this pull request as ready for review Aug 6, 2021
@jimfulton jimfulton requested a review from as a code owner Aug 6, 2021
README.rst Outdated
.. code-block:: python
from pybigquery.api import ApiClient
from sqlalchemy_bigquery.api import ApiClient
api_client = ApiClient()
print(api_client.dry_run_query(query=sqlstr).total_bytes_processed)
Copy link
Collaborator

@tswast tswast Aug 6, 2021

We can remove this code sample now that we don't have an ApiClient

Copy link
Contributor Author

@jimfulton jimfulton Aug 6, 2021

Dang, I did yesterday, but I guess that change lost in my docfx flailing. Thanks, will do.

Copy link
Contributor Author

@jimfulton jimfulton Aug 7, 2021

done

tswast
tswast approved these changes Aug 10, 2021
@jimfulton jimfulton merged commit a6f0a5d into googleapis:master Aug 10, 2021
8 of 9 checks passed
@jimfulton jimfulton deleted the sqlalchemy-bigquery-197 branch Aug 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants