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 Workspace Admin: use SAT #6878

Merged
merged 10 commits into from
Oct 31, 2021
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@
"sourceDefinitionId": "ed9dfefa-1bbc-419d-8c5e-4d78f0ef6734",
"name": "Google Workspace Admin Reports",
"dockerRepository": "airbyte/source-google-workspace-admin-reports",
"dockerImageTag": "0.1.4",
"dockerImageTag": "0.1.5",
"documentationUrl": "https://docs.airbyte.io/integrations/sources/google-workspace-admin-reports"
}
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@
- sourceDefinitionId: ed9dfefa-1bbc-419d-8c5e-4d78f0ef6734
name: Google Workspace Admin Reports
dockerRepository: airbyte/source-google-workspace-admin-reports
dockerImageTag: 0.1.4
dockerImageTag: 0.1.5
documentationUrl: https://docs.airbyte.io/integrations/sources/google-workspace-admin-reports
sourceType: api
- sourceDefinitionId: d1aa448b-7c54-498e-ad95-263cbebcd2db
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,5 @@ RUN pip install .

ENV AIRBYTE_ENTRYPOINT "/airbyte/base.sh"

LABEL io.airbyte.version=0.1.4
LABEL io.airbyte.version=0.1.5
LABEL io.airbyte.name=airbyte/source-google-workspace-admin-reports
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# See [Source Acceptance Tests](https://docs.airbyte.io/connector-development/testing-connectors/source-acceptance-tests-reference)
# for more information about how to configure these tests
connector_image: airbyte/source-google-workspace-admin-reports:dev
tests:
spec:
- spec_path: "source_google_workspace_admin_reports/spec.json"
connection:
- config_path: "secrets/config.json"
status: "succeed"
- config_path: "integration_tests/invalid_config.json"
status: "failed"
discovery:
- config_path: "secrets/config.json"
basic_read:
- config_path: "secrets/config.json"
configured_catalog_path: "integration_tests/configured_catalog.json"
empty_streams: ["admin"]
full_refresh:
- config_path: "secrets/config.json"
configured_catalog_path: "integration_tests/configured_catalog.json"
incremental:
- config_path: "secrets/config.json"
configured_catalog_path: "integration_tests/configured_catalog.json"
future_state_path: "integration_tests/abnormal_state.json"
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#!/usr/bin/env sh

# Build latest connector image
docker build . -t $(cat acceptance-test-config.yml | grep "connector_image" | head -n 1 | cut -d: -f2):dev

# Pull latest acctest image
docker pull airbyte/source-acceptance-test:latest

# Run
docker run --rm -it \
-v /var/run/docker.sock:/var/run/docker.sock \
-v /tmp:/tmp \
-v $(pwd):/test_input \
airbyte/source-acceptance-test \
--acceptance-test-config /test_input

Original file line number Diff line number Diff line change
@@ -1,21 +1,14 @@
plugins {
id 'airbyte-python'
id 'airbyte-docker'
id 'airbyte-standard-source-test-file'
id 'airbyte-source-acceptance-test'
}

airbytePython {
moduleDirectory 'source_google_workspace_admin_reports'
}

airbyteStandardSourceTestFile {
specPath = "source_google_workspace_admin_reports/spec.json"
configPath = "secrets/config.json"
configuredCatalogPath = "sample_files/configured_catalog.json"
}


dependencies {
implementation files(project(':airbyte-integrations:bases:base-standard-source-test-file').airbyteDocker.outputs)
implementation files(project(':airbyte-integrations:bases:source-acceptance-test').airbyteDocker.outputs)
implementation files(project(':airbyte-integrations:bases:base-python').airbyteDocker.outputs)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"drive": {
"time": "2121-04-04T22:01:22.313Z"
},
"oauth_tokens": {
"time": "2121-04-05T03:06:30.849Z"
},
"admin": {
"time": "2121-04-05T03:06:30.849Z"
},
"logins": {
"time": "2121-04-05T03:06:30.849Z"
},
"mobile": {
"time": "2121-04-05T03:06:30.849Z"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#
# Copyright (c) 2021 Airbyte, Inc., all rights reserved.
#


import pytest

pytest_plugins = ("source_acceptance_test.plugin",)


@pytest.fixture(scope="session", autouse=True)
def connector_setup():
"""This fixture is a placeholder for external resources that acceptance test might require."""
yield
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
{
"streams": [
{
"stream": {
"name": "admin",
"json_schema": {},
"supported_sync_modes": ["incremental"],
"source_defined_cursor": true,
"default_cursor_field": ["time"]
},
"sync_mode": "incremental",
"cursor_field": ["time"],
"destination_sync_mode": "append"
},
{
"stream": {
"name": "drive",
"json_schema": {},
"supported_sync_modes": ["incremental"],
"source_defined_cursor": true,
"default_cursor_field": ["time"]
},
"sync_mode": "incremental",
"cursor_field": ["time"],
"destination_sync_mode": "append"
},
{
"stream": {
"name": "logins",
"json_schema": {},
"supported_sync_modes": ["incremental"],
"source_defined_cursor": true,
"default_cursor_field": ["time"]
},
"sync_mode": "incremental",
"cursor_field": ["time"],
"destination_sync_mode": "append"
},
{
"stream": {
"name": "mobile",
"json_schema": {},
"supported_sync_modes": ["incremental"],
"source_defined_cursor": true,
"default_cursor_field": ["time"]
},
"sync_mode": "incremental",
"cursor_field": ["time"],
"destination_sync_mode": "append"
},
{
"stream": {
"name": "oauth_tokens",
"json_schema": {},
"supported_sync_modes": ["incremental"],
"source_defined_cursor": true,
"default_cursor_field": ["time"]
},
"sync_mode": "incremental",
"cursor_field": ["time"],
"destination_sync_mode": "append"
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"credentials_json": "{\n \"type\": \"service_account\"}\n",
"email": "test_email",
"lookback": 0
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# This file is autogenerated -- only edit if you know what you are doing. Use setup.py for declaring dependencies.
-e ../../bases/airbyte-protocol
-e ../../bases/base-python
-e ../../bases/source-acceptance-test
-e .

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@

from setuptools import find_packages, setup

TEST_REQUIREMENTS = [
"pytest~=6.1",
"source-acceptance-test",
]

setup(
name="source_google_workspace_admin_reports",
description="Source implementation for Google Workspace Admin Reports.",
Expand All @@ -14,12 +19,14 @@
install_requires=[
"airbyte-protocol",
"base-python",
"pytest==6.1.2",
"google-api-python-client==2.0.2",
"google-auth-httplib2==0.1.0",
"google-auth-oauthlib==0.4.3",
"backoff==1.10.0",
"pendulum==2.1.2",
],
package_data={"": ["*.json", "schemas/*.json"]},
extras_require={
"tests": TEST_REQUIREMENTS,
},
)
Original file line number Diff line number Diff line change
Expand Up @@ -108,13 +108,13 @@ class IncrementalStreamAPI(StreamAPI, ABC):
def state(self) -> Optional[Mapping[str, Any]]:
"""Current state, if wasn't set return None"""
if self._state:
return {self.state_pk: str(self._state)}
return {self.state_pk: self._state.isoformat()}
return None

@state.setter
def state(self, value):
self._state = pendulum.parse(value[self.state_pk])
self._start_time = self._state.isoformat()
self._start_time = self._state.to_iso8601_string()

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
Expand All @@ -128,6 +128,7 @@ def read(self, getter: Callable, params: Mapping[str, Any] = None) -> Iterator:
"Report API return records from newest to oldest"
if not cursor:
cursor = pendulum.parse(record[self.state_pk])
record[self.state_pk] = pendulum.parse(record[self.state_pk]).isoformat()
yield record

if cursor:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@
"customerId": {
"type": "string"
}
},
"required": ["time", "uniqueQualifier", "applicationName", "customerId"]
}
},
"actor": {
"type": "object",
Expand All @@ -38,8 +37,7 @@
"key": {
"type": "string"
}
},
"required": ["callerType", "email", "profileId", "key"]
}
},
"ownerDomain": {
"type": "string"
Expand All @@ -49,19 +47,20 @@
},
"events": {
"type": "array",
"items": [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here and below the events field has a dynamic structure, that is, this field is returned as an array of objects with different subfields and variable nesting. Is this okay to replace it with an object?

Copy link
Contributor

Choose a reason for hiding this comment

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

Could explain more about "dynamic" schema? Just asking because we are losing a lot of specificity in the data which makes it harder to consume. Are these essentially oneOfs? Was the existing schema incorrect?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sherifnada The parameters field has a list of objects with variable payload (different objects within the same array at the same point of time), in some cases there are deeply nested structures, and the inner items are different from each other, so the oneOf is not applicable, the anyOf possibly could work

Copy link
Contributor

Choose a reason for hiding this comment

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

I am just worried this will cause breaking changes to the schema. This is a pretty important stream (events are potentially the most important resource in this stream) so like described in UX handbook sections Describe output schemas as completely and reliably as possible and Be very cautious about breaking changes to output schemas, it is probably worth attempting to describe the schema as completely as possible. Do you have any insight into how doable this is? @gaart

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sherifnada schema updated, nested fields added

{
"type": "object",
"properties": {
"type": {
"type": "string"
},
"name": {
"type": "string"
},
"parameters": {
"type": "array",
"items": [
"items": {
"type": "object",
"properties": {
"type": {
"type": "string"
},
"name": {
"type": "string"
},
"parameters": {
"type": "array",
"items": {
"type": "object",
"anyOf": [
{
"type": "object",
"properties": {
Expand All @@ -70,22 +69,53 @@
},
"value": {
"type": "string"
}
}
},
{
"type": "object",
"properties": {
"name": {
"type": "string"
},
"intValue": {
"type": "string"
}
}
},
{
"type": "object",
"properties": {
"name": {
"type": "string"
},
"boolValue": {
"type": "boolean"
}
}
},
{
"type": "object",
"properties": {
"name": {
"type": "string"
},
"multiValue": {
Copy link
Contributor

Choose a reason for hiding this comment

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

💪🏼

"type": "array",
"items": {
"type": "string"
}
}
},
"required": ["name", "value", "intValue", "boolValue"]
}
}
]
}
},
"required": ["type", "name", "parameters"]
}
}
]
}
},
"time": {
"type": "string"
}
}
}
Loading