Skip to content

Commit

Permalink
Support display_type property for oneOf fields and adjust CDC connect…
Browse files Browse the repository at this point in the history
…ors to use it (airbytehq#29821)

Co-authored-by: lmossman <lmossman@users.noreply.github.com>
  • Loading branch information
2 people authored and harrytou committed Sep 1, 2023
1 parent f20216a commit 7072201
Show file tree
Hide file tree
Showing 38 changed files with 453 additions and 176 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# Changelog

## 1.0.3
Add tests for display_type property

## 1.0.2
Fix bug in skip_backward_compatibility_tests_fixture, the previous connector version could not be retrieved.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ RUN poetry install --no-root --only main --no-interaction --no-ansi
COPY . /app
RUN poetry install --only main --no-cache --no-interaction --no-ansi

LABEL io.airbyte.version=1.0.2
LABEL io.airbyte.version=1.0.3
LABEL io.airbyte.name=airbyte/connector-acceptance-test
WORKDIR /test_input
ENTRYPOINT ["python", "-m", "pytest", "-p", "connector_acceptance_test.plugin", "-r", "fEsx", "--show-capture=log"]
Original file line number Diff line number Diff line change
Expand Up @@ -96,14 +96,17 @@ You may want to iterate on the acceptance test project itself: adding new tests,
These iterations are more conveniently achieved by remaining in the current directory.

1. Install dependencies via `poetry install`
3. Run the unit tests on the acceptance tests themselves: `poetry run python -m pytest unit_tests` (add the `--pdb` option if you want to enable the debugger on test failure)
2. Run the unit tests on the acceptance tests themselves: `poetry run python -m pytest unit_tests` (add the `--pdb` option if you want to enable the debugger on test failure)
3. To run specific unit test(s), add `-k` to the above command, e.g. `poetry run python -m pytest unit_tests -k 'test_property_can_store_secret'`. You can use wildcards `*` here as well.
4. Make the changes you want:
* Global pytest fixtures are defined in `./connector_acceptance_test/conftest.py`
* Existing test modules are defined in `./connector_acceptance_test/tests`
* `acceptance-test-config.yaml` structure is defined in `./connector_acceptance_test/config.py`
5. Unit test your changes by adding tests to `./unit_tests`
6. Run the unit tests on the acceptance tests again: `poetry run pytest unit_tests`, make sure the coverage did not decrease. You can bypass slow tests by using the `slow` marker: `poetry run pytest unit_tests -m "not slow"`.
7. Manually test the changes you made by running acceptance tests on a specific connector. e.g. `poetry run pytest -p connector_acceptance_test.plugin --acceptance-test-config=../../connectors/source-pokeapi`
7. Manually test the changes you made by running acceptance tests on a specific connector:
* First build the connector to ensure your local image is up-to-date: `./gradlew :airbyte-integrations:connectors:source-pokeapi:airbyteDocker`
* Then run the acceptance tests on the connector: `poetry run pytest -p connector_acceptance_test.plugin --acceptance-test-config=../../connectors/source-pokeapi`
8. Make sure you updated `docs/connector-development/testing-connectors/connector-acceptance-tests-reference.md` according to your changes
9. Bump the acceptance test docker image version in `airbyte-integrations/bases/connector-acceptance-test/Dockerfile`
10. Update the project changelog `airbyte-integrations/bases/connector-acceptance-test/CHANGELOG.md`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -459,27 +459,26 @@ def test_nested_group(self, actual_connector_spec: ConnectorSpecification):
errors.append(f"Groups can only be defined on top level, is defined at {group_path}")
self._fail_on_errors(errors)

def test_required_always_show(self, actual_connector_spec: ConnectorSpecification):
def test_display_type(self, actual_connector_spec: ConnectorSpecification):
"""
Fields with always_show are not allowed to be required fields because only optional fields can be hidden in the form in the first place.
The display_type property can only be set on fields which have a oneOf property, and must be either "dropdown" or "radio"
"""
errors = []
schema_helper = JsonSchemaHelper(actual_connector_spec.connectionSpecification)
for result in dpath.util.search(actual_connector_spec.connectionSpecification, "/properties/**/always_show", yielded=True):
always_show_path = result[0]
parent_path = schema_helper.get_parent_path(always_show_path)
is_property_named_always_show = parent_path.endswith("properties")
if is_property_named_always_show:
for result in dpath.util.search(actual_connector_spec.connectionSpecification, "/properties/**/display_type", yielded=True):
display_type_path = result[0]
parent_path = schema_helper.get_parent_path(display_type_path)
is_property_named_display_type = parent_path.endswith("properties")
if is_property_named_display_type:
continue
property_name = parent_path.rsplit(sep="/", maxsplit=1)[1]
properties_path = schema_helper.get_parent_path(parent_path)
parent_object = schema_helper.get_parent(properties_path)
if (
"required" in parent_object
and isinstance(parent_object.get("required"), List)
and property_name in parent_object.get("required")
):
errors.append(f"always_show is only allowed on optional properties, but is set on {always_show_path}")
parent_object = schema_helper.get_parent(display_type_path)
if not "oneOf" in parent_object:
errors.append(f"display_type is only allowed on fields which have a oneOf property, but is set on {parent_path}")
display_type_value = parent_object.get("display_type")
if display_type_value != "dropdown" and display_type_value != "radio":
errors.append(
f"display_type must be either 'dropdown' or 'radio', but is set to '{display_type_value}' at {display_type_path}"
)
self._fail_on_errors(errors)

def test_defined_refs_exist_in_json_spec_file(self, connector_spec_dict: dict):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1016,40 +1016,123 @@ def test_nested_group(mocker, connector_spec, should_fail):
@pytest.mark.parametrize(
"connector_spec, should_fail",
(
({"type": "object", "properties": {"refresh_token": {"type": "boolean", "airbyte_secret": True}}}, False),
({"type": "object", "properties": {"prop": {"type": "boolean", "airbyte_secret": True, "always_show": True}}}, False),
# SUCCESS: no display_type specified
(
{
"type": "object",
"required": ["prop"],
"properties": {"prop": {"type": "boolean", "airbyte_secret": True, "always_show": True}},
},
True,
"type": "object",
"properties": {
"select_type": {
"type": "object",
"oneOf": [
{
"type": "object",
"properties": {
"option_title": {"type": "string", "title": "Title", "const": "first option"},
"something": {"type": "string"},
},
},
{
"type": "object",
"properties": {
"option_title": {"type": "string", "title": "Title", "const": "second option"},
"some_field": {"type": "boolean"},
},
},
],
},
},
},
False,
),
# SUCCESS: display_type is set to a valid value on a field with oneOf set
(
{
"type": "object",
"properties": {
"select_type": {
"type": "object",
"display_type": "radio",
"oneOf": [
{
"type": "object",
"properties": {
"option_title": {"type": "string", "title": "Title", "const": "first option"},
"something": {"type": "string"},
},
},
{
"type": "object",
"properties": {
"option_title": {"type": "string", "title": "Title", "const": "second option"},
"some_field": {"type": "boolean"},
},
},
],
},
},
},
False,
),
# SUCCESS: display_type is the name of the property
(
{"type": "object", "properties": {"jwt": {"type": "object", "properties": {"a": {"type": "string", "always_show": True}}}}},
{
"type": "object",
"properties": {
"display_type": {
"type": "string",
},
},
},
False,
),
# FAILURE: display_type is set to an invalid value
(
{
"type": "object",
"properties": {"jwt": {"type": "object", "required": ["a"], "properties": {"a": {"type": "string", "always_show": True}}}},
},
"type": "object",
"properties": {
"select_type": {
"type": "object",
"display_type": "invalid",
"oneOf": [
{
"type": "object",
"properties": {
"option_title": {"type": "string", "title": "Title", "const": "first option"},
"something": {"type": "string"},
},
},
{
"type": "object",
"properties": {
"option_title": {"type": "string", "title": "Title", "const": "second option"},
"some_field": {"type": "boolean"},
},
},
],
},
},
},
True,
),
# FAILURE: display_type is set on a non-oneOf field
(
{
"type": "object",
"properties": {"jwt": {"type": "object", "required": ["always_show"], "properties": {"always_show": {"type": "string"}}}},
},
False,
"type": "object",
"properties": {
"select_type": {
"type": "string",
"display_type": "dropdown",
},
},
},
True,
),
),
)
def test_required_always_show(mocker, connector_spec, should_fail):
def test_display_type(mocker, connector_spec, should_fail):
mocker.patch.object(conftest.pytest, "fail")
t = _TestSpec()
t.test_required_always_show(ConnectorSpecification(connectionSpecification=connector_spec))
t.test_display_type(ConnectorSpecification(connectionSpecification=connector_spec))
if should_fail:
conftest.pytest.fail.assert_called_once()
else:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,14 @@ definitions:
primary_key: "id"
$parameters:
path: "workspaces/{{ config['workspace_id'] }}/users"

projects_stream:
$ref: "#/definitions/base_stream"
name: "projects"
primary_key: "id"
$parameters:
path: "workspaces/{{ config['workspace_id'] }}/projects"

clients_stream:
$ref: "#/definitions/base_stream"
name: "clients"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ connectionSpecification:
title: Clockify Spec
type: object
required:
- workspace_id
- api_key
- workspace_id
- api_key
additionalProperties: true
properties:
workspace_id:
Expand All @@ -14,13 +14,15 @@ connectionSpecification:
type: string
api_key:
title: API Key
description: You can get your api access_key <a href="https://app.clockify.me/user/settings">here</a>
description:
You can get your api access_key <a href="https://app.clockify.me/user/settings">here</a>
This API is Case Sensitive.
type: string
airbyte_secret: true
api_url:
title: API Url
description: The URL for the Clockify API. This should only need to be modified
description:
The URL for the Clockify API. This should only need to be modified
if connecting to an enterprise version of Clockify.
type: string
default: https://api.clockify.me
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,5 @@ ENV APPLICATION source-mssql-strict-encrypt

COPY --from=build /airbyte /airbyte

LABEL io.airbyte.version=2.0.0
LABEL io.airbyte.version=2.0.1
LABEL io.airbyte.name=airbyte/source-mssql-strict-encrypt
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ data:
connectorSubtype: database
connectorType: source
definitionId: b5ea17b1-f170-46dc-bc31-cc744ca984c1
dockerImageTag: 2.0.0
dockerImageTag: 2.0.1
dockerRepository: airbyte/source-mssql-strict-encrypt
githubIssueLabel: source-mssql
icon: mssql.svg
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,26 +97,15 @@
},
"replication_method": {
"type": "object",
"title": "Replication Method",
"description": "The replication method used for extracting data from the database. STANDARD replication requires no setup on the DB side but will not be able to represent deletions incrementally. CDC uses {TBC} to detect inserts, updates, and deletes. This needs to be configured on the source database itself.",
"default": "STANDARD",
"title": "Update Method",
"description": "Configures how data is extracted from the database.",
"default": "CDC",
"display_type": "radio",
"order": 8,
"oneOf": [
{
"title": "Standard",
"description": "Standard replication requires no setup on the DB side but will not be able to represent deletions incrementally.",
"required": ["method"],
"properties": {
"method": {
"type": "string",
"const": "STANDARD",
"order": 0
}
}
},
{
"title": "Logical Replication (CDC)",
"description": "CDC uses {TBC} to detect inserts, updates, and deletes. This needs to be configured on the source database itself.",
"title": "Read Changes using Change Data Capture (CDC)",
"description": "<i>Recommended</i> - Incrementally reads new inserts, updates, and deletes using the SQL Server's <a href=\"https://docs.airbyte.com/integrations/sources/mssql/#change-data-capture-cdc\">change data capture feature</a>. This must be enabled on your database.",
"required": ["method"],
"properties": {
"method": {
Expand Down Expand Up @@ -150,6 +139,18 @@
"order": 3
}
}
},
{
"title": "Scan Changes with User Defined Cursor",
"description": "Incrementally detects new inserts and updates using the <a href=\"https://docs.airbyte.com/understanding-airbyte/connections/incremental-append/#user-defined-cursor\">cursor column</a> chosen when configuring a connection (e.g. created_at, updated_at).",
"required": ["method"],
"properties": {
"method": {
"type": "string",
"const": "STANDARD",
"order": 0
}
}
}
]
}
Expand Down
2 changes: 1 addition & 1 deletion airbyte-integrations/connectors/source-mssql/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,5 @@ ENV APPLICATION source-mssql

COPY --from=build /airbyte /airbyte

LABEL io.airbyte.version=2.0.0
LABEL io.airbyte.version=2.0.1
LABEL io.airbyte.name=airbyte/source-mssql
2 changes: 1 addition & 1 deletion airbyte-integrations/connectors/source-mssql/metadata.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ data:
connectorSubtype: database
connectorType: source
definitionId: b5ea17b1-f170-46dc-bc31-cc744ca984c1
dockerImageTag: 2.0.0
dockerImageTag: 2.0.1
dockerRepository: airbyte/source-mssql
documentationUrl: https://docs.airbyte.com/integrations/sources/mssql
githubIssueLabel: source-mssql
Expand Down
Loading

0 comments on commit 7072201

Please sign in to comment.