fix malformed URL causing 'since' filter to be dropped for Issues and Comments streams

CHANGELOG.md

@@ -1,5 +1,8 @@
 # Changelog
 
+# 3.3.4
+  * Fix malformed URL causing 'since' filter to be dropped for Issues and Comments streams [#230](https://github.com/singer-io/tap-github/pull/230)
+
 # 3.3.3
   * Fix `translate_state` to not delete state [#224](https://github.com/singer-io/tap-github/pull/224)
     * Add unit tests to prove there's a bug and prevent future regression

setup.py

@@ -3,15 +3,15 @@
 from setuptools import setup, find_packages
 
 setup(name='tap-github',
-      version='3.3.3',
+      version='3.3.4',
       description='Singer.io tap for extracting data from the GitHub API',
       author='Stitch',
       url='http://singer.io',
       classifiers=['Programming Language :: Python :: 3 :: Only'],
       py_modules=['tap_github'],
       install_requires=[
           'singer-python==5.12.1',
-          'requests==2.32.4',
+          'requests==2.33.0',
           'backoff==1.8.0'
       ],
       extras_require={

tap_github/streams.py

@@ -74,8 +74,9 @@ def build_url(self, base_url, repo_path, bookmark):
         Build the full url with parameters and attributes.
         """
         if self.filter_param:
-            # Add the since parameter for incremental streams
-            query_string = '?since={}'.format(bookmark)
+            # Use '&' if the path already contains a query string, otherwise '?'
+            separator = '&' if '?' in (self.path or '') else '?'
+            query_string = '{}since={}'.format(separator, bookmark)
         else:
             query_string = ''
 
@@ -387,6 +388,7 @@ def sync_endpoint(self,
             ):
                 records = response.json()
                 extraction_time = singer.utils.now()
+
                 for record in records:
                     record['_sdc_repository'] = repo_path
                     self.add_fields_at_1st_level(record = record, parent_record = None)

tests/unittests/test_stream.py

@@ -1,6 +1,6 @@
 import unittest
 from unittest import mock
-from tap_github.streams import Comments, Reviews, TeamMemberships, Teams, PullRequests, get_schema, get_child_full_url, get_bookmark
+from tap_github.streams import Comments, Reviews, TeamMemberships, Teams, PullRequests, Commits, Issues, get_schema, get_child_full_url, get_bookmark
 from parameterized import parameterized
 
 
@@ -62,8 +62,21 @@ class TestBuildUrl(unittest.TestCase):
     """
 
     @parameterized.expand([
-        ["test_stream_with_filter_params", "org/test-repo", "https://api.github.com/repos/org/test-repo/issues/comments?sort=updated&direction=desc?since=2022-01-01T00:00:00Z", Comments],
-        ["test_stream_with_organization", "org", "https://api.github.com/orgs/org/teams", Teams]
+        # Path already has '?' — since must be appended with '&'
+        ["comments_appends_ampersand_since", "org/test-repo",
+         "https://api.github.com/repos/org/test-repo/issues/comments?sort=updated&direction=desc&since=2022-01-01T00:00:00Z",
+         Comments],
+        ["issues_appends_ampersand_since", "org/test-repo",
+         "https://api.github.com/repos/org/test-repo/issues?state=all&sort=updated&direction=desc&since=2022-01-01T00:00:00Z",
+         Issues],
+        # Path has no '?' — since must be appended with '?'
+        ["commits_appends_question_mark_since", "org/test-repo",
+         "https://api.github.com/repos/org/test-repo/commits?since=2022-01-01T00:00:00Z",
+         Commits],
+        # Org-level stream with no filter_param — no since appended
+        ["teams_org_url_no_since", "org",
+         "https://api.github.com/orgs/org/teams",
+         Teams],
     ])
     def test_build_url(self, name, param, expected_url, stream_class):
         """