Skip to content

Commit 6e6cc5c

Browse files
authored
fix tagging for api sources (required reviewers) (#37728)
## What * Handle tagging for API sources ## How * critical connectors auto-tagged via CODEOWNERS * other api sources not auto-tagged at the moment, we can re-evaluate if we want it back ## Can this PR be safely reverted and rolled back? <!-- * If unsure, leave it blank. --> - [x] YES 💚 - [ ] NO ❌
1 parent 9c4e38d commit 6e6cc5c

File tree

3 files changed

+14
-82
lines changed

3 files changed

+14
-82
lines changed

.github/CODEOWNERS

+7
Original file line numberDiff line numberDiff line change
@@ -51,3 +51,10 @@
5151
/airbyte-integrations/connectors/destination-s3/ @airbytehq/destinations
5252
/airbyte-integrations/connectors/destination-snowflake/ @airbytehq/destinations
5353
/airbyte-integrations/connectors/destination-redshift/ @airbytehq/destinations
54+
55+
# Python critical connectors
56+
/airbyte-integrations/connectors/source-facebook-marketing/ @airbytehq/critical-connectors
57+
/airbyte-integrations/connectors/source-hubspot/ @airbytehq/critical-connectors
58+
/airbyte-integrations/connectors/source-salesforce/ @airbytehq/critical-connectors
59+
/airbyte-integrations/connectors/source-shopify/ @airbytehq/critical-connectors
60+
/airbyte-integrations/connectors/source-stripe/ @airbytehq/critical-connectors

airbyte-ci/connectors/connector_ops/connector_ops/required_reviewer_checks.py

-24
Original file line numberDiff line numberDiff line change
@@ -7,37 +7,13 @@
77
import yaml
88
from connector_ops import utils
99

10-
# TODO: Check to see if GL still wants to get tagged in this. If so,
11-
# the logic needs to be audited to make sure its actually just python.
12-
STRATEGIC_PYTHON_CONNECTOR_REVIEWERS = {"gl-python"}
1310
# The breaking change reviewers is still in active use.
1411
BREAKING_CHANGE_REVIEWERS = {"breaking-change-reviewers"}
1512
REVIEW_REQUIREMENTS_FILE_PATH = ".github/connector_org_review_requirements.yaml"
1613

1714

18-
def find_changed_strategic_connectors(
19-
languages: Tuple[utils.ConnectorLanguage] = (
20-
utils.ConnectorLanguage.JAVA,
21-
utils.ConnectorLanguage.LOW_CODE,
22-
utils.ConnectorLanguage.PYTHON,
23-
)
24-
) -> Set[utils.Connector]:
25-
"""Find important connectors modified on the current branch.
26-
27-
Returns:
28-
Set[utils.Connector]: The set of important connectors that were modified on the current branch.
29-
"""
30-
changed_connectors = utils.get_changed_connectors(destination=False, third_party=False)
31-
return {connector for connector in changed_connectors if connector.is_strategic_connector and connector.language in languages}
32-
33-
3415
def find_mandatory_reviewers() -> List[Dict[str, Union[str, Dict[str, List]]]]:
3516
requirements = [
36-
{
37-
"name": "Strategic python connectors",
38-
"teams": list(STRATEGIC_PYTHON_CONNECTOR_REVIEWERS),
39-
"is_required": find_changed_strategic_connectors((utils.ConnectorLanguage.PYTHON, utils.ConnectorLanguage.LOW_CODE)),
40-
},
4117
{
4218
"name": "Breaking changes",
4319
"teams": list(BREAKING_CHANGE_REVIEWERS),

airbyte-ci/connectors/connector_ops/tests/test_required_reviewer_checks.py

+7-58
Original file line numberDiff line numberDiff line change
@@ -19,46 +19,20 @@ def mock_diffed_branched(mocker):
1919
return airbyte_repo.active_branch
2020

2121

22-
@pytest.fixture
23-
def pokeapi_acceptance_test_config_path():
24-
return "airbyte-integrations/connectors/source-pokeapi/acceptance-test-config.yml"
25-
26-
2722
@pytest.fixture
2823
def pokeapi_metadata_path():
2924
return "airbyte-integrations/connectors/source-pokeapi/metadata.yaml"
3025

3126

3227
@pytest.fixture
33-
def strategic_connector_file():
34-
return "airbyte-integrations/connectors/source-amplitude/acceptance-test-config.yml"
35-
36-
37-
@pytest.fixture
38-
def strategic_connector_metadata_path():
39-
return "airbyte-integrations/connectors/source-amplitude/metadata.yaml"
40-
41-
42-
@pytest.fixture
43-
def not_strategic_not_tracked_change_expected_team(tmp_path, pokeapi_acceptance_test_config_path):
28+
def not_tracked_change_expected_team(tmp_path, pokeapi_metadata_path):
4429
expected_teams = []
4530
backup_path = tmp_path / "non_strategic_acceptance_test_config.backup"
46-
shutil.copyfile(pokeapi_acceptance_test_config_path, backup_path)
47-
with open(pokeapi_acceptance_test_config_path, "a") as acceptance_test_config_file:
48-
acceptance_test_config_file.write("not_tracked")
49-
yield expected_teams
50-
shutil.copyfile(backup_path, pokeapi_acceptance_test_config_path)
51-
52-
53-
@pytest.fixture
54-
def strategic_connector_file_change_expected_team(tmp_path, strategic_connector_file):
55-
expected_teams = list(required_reviewer_checks.STRATEGIC_PYTHON_CONNECTOR_REVIEWERS)
56-
backup_path = tmp_path / "strategic_acceptance_test_config.backup"
57-
shutil.copyfile(strategic_connector_file, backup_path)
58-
with open(strategic_connector_file, "a") as strategic_acceptance_test_config_file:
59-
strategic_acceptance_test_config_file.write("foobar")
31+
shutil.copyfile(pokeapi_metadata_path, backup_path)
32+
with open(pokeapi_metadata_path, "a") as metadata_file:
33+
metadata_file.write("not_tracked")
6034
yield expected_teams
61-
shutil.copyfile(backup_path, strategic_connector_file)
35+
shutil.copyfile(backup_path, pokeapi_metadata_path)
6236

6337

6438
@pytest.fixture
@@ -74,21 +48,6 @@ def test_breaking_change_release_expected_team(tmp_path, pokeapi_metadata_path)
7448
shutil.copyfile(backup_path, pokeapi_metadata_path)
7549

7650

77-
@pytest.fixture
78-
def strategic_connector_breaking_change_release_expected_teams(tmp_path, strategic_connector_metadata_path):
79-
expected_teams = list(
80-
required_reviewer_checks.STRATEGIC_PYTHON_CONNECTOR_REVIEWERS.union(required_reviewer_checks.BREAKING_CHANGE_REVIEWERS)
81-
)
82-
backup_path = tmp_path / "strategic_acceptance_test_config.backup"
83-
shutil.copyfile(strategic_connector_metadata_path, backup_path)
84-
with open(strategic_connector_metadata_path, "a") as strategic_connector_metadata_file:
85-
strategic_connector_metadata_file.write(
86-
"releases:\n breakingChanges:\n 23.0.0:\n message: hi\n upgradeDeadline: 2025-01-01"
87-
)
88-
yield expected_teams
89-
shutil.copyfile(backup_path, strategic_connector_metadata_path)
90-
91-
9251
def verify_no_requirements_file_was_generated(captured: str):
9352
assert captured.out.split("\n")[0].split("=")[-1] == "false"
9453

@@ -115,19 +74,9 @@ def check_review_requirements_file(capsys, expected_teams: List):
11574
verify_review_requirements_file_contains_expected_teams(requirements_file_path, expected_teams)
11675

11776

118-
def test_find_mandatory_reviewers_ga(capsys, strategic_connector_file_change_expected_team):
119-
check_review_requirements_file(capsys, strategic_connector_file_change_expected_team)
120-
121-
12277
def test_find_mandatory_reviewers_breaking_change_release(capsys, test_breaking_change_release_expected_team):
12378
check_review_requirements_file(capsys, test_breaking_change_release_expected_team)
12479

12580

126-
def test_find_mandatory_reviewers_no_tracked_changed(capsys, not_strategic_not_tracked_change_expected_team):
127-
check_review_requirements_file(capsys, not_strategic_not_tracked_change_expected_team)
128-
129-
130-
def test_find_mandatory_reviewers_strategic_connector_breaking_change_release(
131-
capsys, strategic_connector_breaking_change_release_expected_teams
132-
):
133-
check_review_requirements_file(capsys, strategic_connector_breaking_change_release_expected_teams)
81+
def test_find_mandatory_reviewers_no_tracked_changed(capsys, not_tracked_change_expected_team):
82+
check_review_requirements_file(capsys, not_tracked_change_expected_team)

0 commit comments

Comments
 (0)