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

Checkout entire git history in test-command #22184

Merged
merged 11 commits into from
Feb 7, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 1 addition & 2 deletions .github/workflows/test-command.yml
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,7 @@ jobs:
retention-days: 3
- name: Run QA checks for ${{ github.event.inputs.connector }}
id: qa_checks
# TODO: Disabled for on master until #22127 resolved
if: contains(github.ref, 'feat-qa-engine')
if: always()
run: |
run-qa-checks ${{ github.event.inputs.connector }}
- name: Report Status
Expand Down
12 changes: 1 addition & 11 deletions tools/ci_connector_ops/ci_connector_ops/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,15 @@
# Copyright (c) 2022 Airbyte, Inc., all rights reserved.
#


from dataclasses import dataclass
import logging
from pathlib import Path
from typing import Dict, Optional, Set, Tuple, List
import os
import git
import requests
import yaml

# ensure we are at the repository root
os.chdir(os.path.dirname(os.path.abspath(__file__)))
os.chdir('../../..')

AIRBYTE_REPO = git.Repo(".")
AIRBYTE_REPO = git.Repo(search_parent_directories=True)
Comment on lines -19 to +13
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, of course - the magic option was set to False...
Would this still work if we added back the os.chdir('../../..')?

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 would not unfortunately.

The issue here came down to the file found it self in different directories and running contexts.

So change the directory via a relative path would always break one of local or ec2

On local the paths were:

  • current working directory: /Users/ben/Development/repos/airbyte/tools/ci_connector_ops
  • file directory: /Users/ben/Development/repos/airbyte/tools/ci_connector_ops/ci_connector_ops

On the ec2 instance the paths were (taken from a workflow we ran):

  • The current working directory was /actions-runner/_work/airbyte/airbyte
  • And the files path was /actions-runner/_work/_tool/Python/3.9.16/x64/lib/python3.9/site-packages/ci_connector_ops/utils.py

So our best way seems to be relying on using the GitPython library to simply traverse up the directory structure until it finds a .git folder

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we shall pass this repo path as an env var for a more explicit definition

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤔 Thinking on that a bit. Perhaps if theres is a case where we would not be inside a git versioned directory.

@alafanechere Is there a context that might happen?

DIFFED_BRANCH = "origin/master"
OSS_CATALOG_URL = "https://storage.googleapis.com/prod-airbyte-cloud-connector-metadata-service/oss_catalog.json"
CONNECTOR_PATH_PREFIX = "airbyte-integrations/connectors"
Expand All @@ -27,15 +21,12 @@
DESTINATION_DEFINITIONS_FILE_PATH = "airbyte-config/init/src/main/resources/seed/destination_definitions.yaml"
DEFINITIONS_FILE_PATH = {"source": SOURCE_DEFINITIONS_FILE_PATH, "destination": DESTINATION_DEFINITIONS_FILE_PATH}


def download_catalog(catalog_url):
response = requests.get(catalog_url)
return response.json()


OSS_CATALOG = download_catalog(OSS_CATALOG_URL)


class ConnectorInvalidNameError(Exception):
pass

Expand All @@ -49,7 +40,6 @@ def read_definitions(definitions_file_path: str) -> Dict:
def get_connector_name_from_path(path):
return path.split("/")[2]


def get_changed_acceptance_test_config(diff_regex: Optional[str]=None) -> Set[str]:
"""Retrieve a list of connector names for which the acceptance_test_config file was changed in the current branch (compared to master).

Expand Down
5 changes: 1 addition & 4 deletions tools/status/report.sh
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,7 @@ CONNECTOR=$1
REPOSITORY=$2
RUN_ID=$3
TEST_OUTCOME=$4

# TODO: Disabled for on master until #22127 resolved
# QA_CHECKS_OUTCOME=$5
QA_CHECKS_OUTCOME=success
QA_CHECKS_OUTCOME=$5

# Ensure connector is prefixed with connectors/
# TODO (ben): In the future we should just hard error if this is not the case
Expand Down