Skip to content
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
190 changes: 98 additions & 92 deletions tap_github/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
debugpy.wait_for_client()
breakpoint()

from minware_singer_utils import GitLocal, SecureLogger
from minware_singer_utils import GitLocal, GitLocalRepoNotFoundException, SecureLogger

from singer import metadata

Expand Down Expand Up @@ -3141,100 +3141,106 @@ def schemaSortFunc(val):
logger.warning(f'{repo} is not available, skipping')
continue

gitLocal = GitLocal({
'access_token': access_token,
'workingDir': '/tmp'
}, 'https://x-access-token:{}@github.com/{}.git',
config['hmac_token'] if 'hmac_token' in config else None,
logger=logger,
commitsOnly=commits_only)

for stream in catalog['streams']:
stream_id = stream['tap_stream_id']
stream_schema = stream['schema']
mdata = stream['metadata']

# if it is a "sub_stream", it will be sync'd by its parent
if not SYNC_FUNCTIONS.get(stream_id):
continue

# if stream is selected, write schema and sync
if stream_id in selected_stream_ids:
logger.info("Syncing stream: %s", stream_id)
write_schema(stream_id, stream_schema, stream['key_properties'])

# get sync function and any sub streams
sync_func = SYNC_FUNCTIONS[stream_id]
sub_stream_ids = SUB_STREAMS.get(stream_id, None)
try:
gitLocal = GitLocal({
'access_token': access_token,
'workingDir': '/tmp'
}, 'https://x-access-token:{}@github.com/{}.git',
config['hmac_token'] if 'hmac_token' in config else None,
logger=logger,
commitsOnly=commits_only)

for stream in catalog['streams']:
stream_id = stream['tap_stream_id']
stream_schema = stream['schema']
mdata = stream['metadata']

# if it is a "sub_stream", it will be sync'd by its parent
if not SYNC_FUNCTIONS.get(stream_id):
continue

# sync stream
if not sub_stream_ids:
if stream_id == 'commit_files' or stream_id == 'commit_files_meta':
commits_only = stream_id == 'commit_files_meta'
stream_schemas = {stream_id: stream_schema}
logger.info(
'SYNC_FUNCTIONS[%s] (%s, %s, %s, %s)',
stream_id,
repo,
start_date,
commits_only,
selected_stream_ids
)
state = sync_func(stream_schemas, repo, state, mdata, start_date, gitLocal, commits_only, selected_stream_ids)
# if stream is selected, write schema and sync
if stream_id in selected_stream_ids:
logger.info("Syncing stream: %s", stream_id)
write_schema(stream_id, stream_schema, stream['key_properties'])

# get sync function and any sub streams
sync_func = SYNC_FUNCTIONS[stream_id]
sub_stream_ids = SUB_STREAMS.get(stream_id, None)

# sync stream
if not sub_stream_ids:
if stream_id == 'commit_files' or stream_id == 'commit_files_meta':
commits_only = stream_id == 'commit_files_meta'
stream_schemas = {stream_id: stream_schema}
logger.info(
'SYNC_FUNCTIONS[%s] (%s, %s, %s, %s)',
stream_id,
repo,
start_date,
commits_only,
selected_stream_ids
)
state = sync_func(stream_schemas, repo, state, mdata, start_date, gitLocal, commits_only, selected_stream_ids)
else:
logger.info(
'SYNC_FUNCTIONS[%s] (%s, %s)',
stream_id,
repo,
start_date
)
state = sync_func(stream_schema, repo, state, mdata, start_date)

# handle streams with sub streams
else:
logger.info(
'SYNC_FUNCTIONS[%s] (%s, %s)',
stream_id,
repo,
start_date
)
state = sync_func(stream_schema, repo, state, mdata, start_date)
stream_schemas = {stream_id: stream_schema}

# handle streams with sub streams
else:
stream_schemas = {stream_id: stream_schema}

# get and write selected sub stream schemas
for sub_stream_id in sub_stream_ids:
if sub_stream_id in selected_stream_ids:
sub_stream = get_stream_from_catalog(sub_stream_id, catalog)
stream_schemas[sub_stream_id] = sub_stream['schema']
write_schema(sub_stream_id, sub_stream['schema'],
sub_stream['key_properties'])

# sync stream and its sub streams
if stream_id == 'commit_files' or stream_id == 'commit_files_meta':
logger.info(
'SYNC_FUNCTIONS[%s] (%s, %s, %s, %s)',
stream_id,
repo,
start_date,
commits_only,
selected_stream_ids
)
state = sync_func(stream_schemas, repo, state, mdata, start_date,
gitLocal, commits_only, selected_stream_ids)
else:
logger.info(
'SYNC_FUNCTIONS[%s] (%s, %s)',
stream_id,
repo,
start_date
)
state = sync_func(stream_schemas, repo, state, mdata, start_date)
# Write the state after each repo. There use to be a check for:
# stream_id != 'branches' and stream_id != 'pull_requests'
# to avoid saving the state after branches or pull_requests and having a data dependency on
# commits reading from their output, but that's no longer necessary that we wait for the
# whole repo to process.
# The reason for writing only after the whole repo is that the state size can get pretty
# big, which will end up exhausting memory in the target due to buffering those state lines
# while it is waiting for a certain amount of data to arrive.
# In the future, we should take a two-pronged appraoch to fixing this of both (1) reducing
# the size of the state itself, and (2) forking and modifying the postgres target to count
# the size of the state it is buffering as part of its memory limits, which it's not doing
# right now and running out of memory as a result.
singer.write_state(state)
# get and write selected sub stream schemas
for sub_stream_id in sub_stream_ids:
if sub_stream_id in selected_stream_ids:
sub_stream = get_stream_from_catalog(sub_stream_id, catalog)
stream_schemas[sub_stream_id] = sub_stream['schema']
write_schema(sub_stream_id, sub_stream['schema'],
sub_stream['key_properties'])

# sync stream and its sub streams
if stream_id == 'commit_files' or stream_id == 'commit_files_meta':
logger.info(
'SYNC_FUNCTIONS[%s] (%s, %s, %s, %s)',
stream_id,
repo,
start_date,
commits_only,
selected_stream_ids
)
state = sync_func(stream_schemas, repo, state, mdata, start_date,
gitLocal, commits_only, selected_stream_ids)
else:
logger.info(
'SYNC_FUNCTIONS[%s] (%s, %s)',
stream_id,
repo,
start_date
)
state = sync_func(stream_schemas, repo, state, mdata, start_date)

# Write the state after each repo. There use to be a check for:
# stream_id != 'branches' and stream_id != 'pull_requests'
# to avoid saving the state after branches or pull_requests and having a data dependency on
# commits reading from their output, but that's no longer necessary that we wait for the
# whole repo to process.
# The reason for writing only after the whole repo is that the state size can get pretty
# big, which will end up exhausting memory in the target due to buffering those state lines
# while it is waiting for a certain amount of data to arrive.
# In the future, we should take a two-pronged appraoch to fixing this of both (1) reducing
# the size of the state itself, and (2) forking and modifying the postgres target to count
# the size of the state it is buffering as part of its memory limits, which it's not doing
# right now and running out of memory as a result.
singer.write_state(state)

except GitLocalRepoNotFoundException as e:
logger.warning(f'Repository {repo} not found, skipping: {e}')
continue



Expand Down
123 changes: 123 additions & 0 deletions tests/unittests/test_repo_not_found.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
import unittest
from unittest import mock
from minware_singer_utils import GitLocalRepoNotFoundException
import tap_github


class TestRepoNotFoundHandling(unittest.TestCase):
"""Test that GitLocalRepoNotFoundException is caught and skipped in do_sync."""

def _make_catalog(self, stream_ids):
"""Build a minimal catalog with the given stream IDs selected."""
streams = []
for sid in stream_ids:
streams.append(
{
"tap_stream_id": sid,
"schema": {},
"key_properties": ["id"],
"metadata": [{"breadcrumb": [], "metadata": {"selected": True}}],
}
)
return {"streams": streams}

@mock.patch("tap_github.set_auth_headers", return_value="fake-token")
@mock.patch("tap_github.get_repo_metadata")
@mock.patch("tap_github.GitLocal")
@mock.patch("tap_github.get_selected_streams", return_value=["repositories"])
@mock.patch("tap_github.validate_dependencies")
@mock.patch("singer.write_state")
@mock.patch.dict(
tap_github.SYNC_FUNCTIONS, {"repositories": mock.MagicMock(return_value={})}
)
def test_skips_repo_when_not_found_on_clone(
self,
mock_write_state,
mock_validate,
mock_get_selected,
mock_git_local_cls,
mock_get_repo_metadata,
mock_set_auth,
):
"""When GitLocal() raises GitLocalRepoNotFoundException, the repo is
skipped and sync continues to the next repo."""
mock_git_local_cls.side_effect = [
GitLocalRepoNotFoundException("repo not found"),
mock.MagicMock(), # second repo succeeds
]

config = {
"repository": "org/missing-repo org/good-repo",
"access_token": "token",
"start_date": "2024-01-01",
}
catalog = self._make_catalog(["repositories"])

# Should not raise — the missing repo is skipped
tap_github.do_sync(config, {}, catalog)

# GitLocal was called for both repos
self.assertEqual(mock_git_local_cls.call_count, 2)

@mock.patch("tap_github.set_auth_headers", return_value="fake-token")
@mock.patch("tap_github.get_repo_metadata")
@mock.patch("tap_github.GitLocal")
@mock.patch("tap_github.get_selected_streams", return_value=["repositories"])
@mock.patch("tap_github.validate_dependencies")
@mock.patch("singer.write_state")
def test_state_not_written_for_missing_repo(
self,
mock_write_state,
mock_validate,
mock_get_selected,
mock_git_local_cls,
mock_get_repo_metadata,
mock_set_auth,
):
"""State should not be written for a repo that was skipped."""
mock_git_local_cls.side_effect = GitLocalRepoNotFoundException("repo not found")

config = {
"repository": "org/missing-repo",
"access_token": "token",
"start_date": "2024-01-01",
}
catalog = self._make_catalog(["repositories"])

tap_github.do_sync(config, {}, catalog)

mock_write_state.assert_not_called()

@mock.patch("tap_github.set_auth_headers", return_value="fake-token")
@mock.patch("tap_github.get_repo_metadata")
@mock.patch("tap_github.GitLocal")
@mock.patch("tap_github.get_selected_streams", return_value=["repositories"])
@mock.patch("tap_github.validate_dependencies")
@mock.patch("singer.write_state")
def test_other_gitlocal_exceptions_still_raise(
self,
mock_write_state,
mock_validate,
mock_get_selected,
mock_git_local_cls,
mock_get_repo_metadata,
mock_set_auth,
):
"""A generic GitLocalException (not repo-not-found) should still propagate."""
from minware_singer_utils import GitLocalException

mock_git_local_cls.side_effect = GitLocalException("network timeout")

config = {
"repository": "org/some-repo",
"access_token": "token",
"start_date": "2024-01-01",
}
catalog = self._make_catalog(["repositories"])

with self.assertRaises(GitLocalException):
tap_github.do_sync(config, {}, catalog)


if __name__ == "__main__":
unittest.main()