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

Source Google Analytics Data API: prepare connector to сertification #20769

Conversation

roman-yermilov-gl
Copy link
Contributor

@roman-yermilov-gl roman-yermilov-gl commented Dec 21, 2022

What

  • Added validation for reports defined by user to avoid errors like KeyError 'name'
  • Added 429 error handling: return empty data and keep going
  • Streams without 'date' dimension are considered to be a full refresh streams
  • Fixed type and description of date_ranges_start_date field

from source_google_analytics_data_api.authenticator import GoogleServiceKeyAuthenticator


private_key = """
Copy link
Collaborator

Choose a reason for hiding this comment

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

it looks like a real key, can we replace it with some kind of dummy string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What's wrong with it looks like a real key ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

it makes me think it is real

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Of course it is real. But again, what is wrong with it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Private keys should not be committed to public repositories

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 private key does not belong to any Airbyte account or any other account in the Universe

Copy link
Collaborator

Choose a reason for hiding this comment

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

Only you know it 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does it matter if only I know it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

well in case this remains in the code and gets merged, I bet I'm not the last person to ask about this key - so the answer is yes

Copy link
Contributor Author

@roman-yermilov-gl roman-yermilov-gl Dec 22, 2022

Choose a reason for hiding this comment

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

I will uncover the mystery of the origin of this private key to every reviewer, or they can solve it by themselves by reading our discussion

"""


def test_authenticator(mocker):
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: requests_mock is a better option to mock http requests, since is less erroneous (see #18931)

@roman-yermilov-gl roman-yermilov-gl force-pushed the ryermilov/source-google-analytics-data-api-to-beta branch from f09437a to 9841bb3 Compare December 23, 2022 13:21
@roman-yermilov-gl roman-yermilov-gl linked an issue Dec 23, 2022 that may be closed by this pull request
@roman-yermilov-gl roman-yermilov-gl linked an issue Dec 23, 2022 that may be closed by this pull request
@roman-yermilov-gl roman-yermilov-gl force-pushed the ryermilov/source-google-analytics-data-api-to-beta branch from 9841bb3 to b0c57b1 Compare December 23, 2022 13:27
@roman-yermilov-gl roman-yermilov-gl changed the title Source Google Analytics Data API: improve acceptance tests; unit test coverage 90% Source Google Analytics Data API: prepare connector to sertification Dec 23, 2022
@roman-yermilov-gl roman-yermilov-gl force-pushed the ryermilov/source-google-analytics-data-api-to-beta branch from b0c57b1 to 8b03744 Compare December 23, 2022 16:05
@roman-yermilov-gl roman-yermilov-gl changed the title Source Google Analytics Data API: prepare connector to sertification Source Google Analytics Data API: prepare connector to сertification Dec 23, 2022
- Added validation for reports defined by user to avoid errors like KeyError 'name'
- Added 429 error handling: return empty data and keep going
- Streams without 'date' dimension are considered to be a full refresh streams
- Fixed type and description of `date_ranges_start_date` field
@roman-yermilov-gl roman-yermilov-gl force-pushed the ryermilov/source-google-analytics-data-api-to-beta branch from 8b03744 to 4ee2e54 Compare December 23, 2022 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unit tests for Google Analytics Data API connector
4 participants