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

[KED-1876] Allow GBQTableDataSet to optionally accept a sql query to load data #443

Conversation

ajb7
Copy link
Contributor

@ajb7 ajb7 commented Jul 22, 2020

Description

Modify current implementation of GBQTableDataSet to accept custom queries to be executed in GBQ to load custom results.

Addresses: #442

Development notes

Modified the _load() method in GBQTableDataSet to accept custom queries passed via load_args in catalog.yml. Eventually passing the arguments in pandas.read_gbq()

Checklist

  • [ X] Read the contributing guidelines
  • [ X] Opened this PR as a 'Draft Pull Request' if it is work-in-progress
  • [ X] Updated the documentation to reflect the code changes
  • [ X] Added a description of this change and added my name to the list of supporting contributions in the RELEASE.md file
  • [ X] Added tests to cover my changes

Notice

  • [X ] I acknowledge and agree that, by checking this box and clicking "Submit Pull Request":

  • I submit this contribution under the Apache 2.0 license and represent that I am entitled to do so on behalf of myself, my employer, or relevant third parties, as applicable.

  • I certify that (a) this contribution is my original creation and / or (b) to the extent it is not my original creation, I am authorised to submit this contribution on behalf of the original creator(s) or their licensees.

  • I certify that the use of this contribution as authorised by the Apache 2.0 license does not violate the intellectual property rights of anyone else.

@ajb7 ajb7 requested a review from idanov as a code owner July 22, 2020 00:05
Copy link
Contributor

@mzjp2 mzjp2 left a comment

Choose a reason for hiding this comment

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

This is really close to ready, thank you for the contribution! 🥳

Some minor comments that you need to address then I think we're good to go. Happy to support you whilst addressing the comments as well!

kedro/extras/datasets/pandas/gbq_dataset.py Outdated Show resolved Hide resolved
kedro/extras/datasets/pandas/gbq_dataset.py Outdated Show resolved Hide resolved
kedro/extras/datasets/pandas/gbq_dataset.py Outdated Show resolved Hide resolved
tests/extras/datasets/pandas/test_gbq_dataset.py Outdated Show resolved Hide resolved
tests/extras/datasets/pandas/test_gbq_dataset.py Outdated Show resolved Hide resolved
@mzjp2 mzjp2 changed the title Feature/modified gbqtabledataset to accept custom queries [KED-1876] Allow GBQTableDataSet to optionally accept a sql query to load data Jul 23, 2020
@ajb7
Copy link
Contributor Author

ajb7 commented Jul 23, 2020

  • Thank you for reviewing 😄
  • I have added parameterized test case for load_args['query']
  • seems like stringify, try/except, etc. were not needed and already taken care of. Reverted them 😬

Copy link
Contributor

@mzjp2 mzjp2 left a comment

Choose a reason for hiding this comment

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

Awesome work! 🎉

loaded_data = gbq_dataset.load()

mocked_read_gbq.assert_called_once_with(
project_id=PROJECT, credentials=None, query=load_args["query"]
Copy link
Contributor

@mzjp2 mzjp2 Jul 23, 2020

Choose a reason for hiding this comment

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

I’d check query=“Select 1” explicitly here but up to you!

@mzjp2 mzjp2 requested a review from limdauto July 23, 2020 22:06
@ajb7 ajb7 requested a review from mzjp2 July 27, 2020 14:51
Copy link
Contributor

@DmitriiDeriabinQB DmitriiDeriabinQB left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@mzjp2 mzjp2 merged commit d6291dc into kedro-org:master Jul 29, 2020
@mzjp2
Copy link
Contributor

mzjp2 commented Jul 29, 2020

Thanks again @ajb7

@ajb7
Copy link
Contributor Author

ajb7 commented Jul 29, 2020

Thank you Kedro Team!!! 😄 ⭐

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