TDL-6737 added pylint and resolved the errors #87

namrata270998 · 2021-10-07T08:43:31Z

Description of change

Added pylint in the circleci and fixed its corresponding errors

Manual QA steps

Risks

Rollback steps

revert this branch

dbshah1212 · 2021-10-07T10:44:35Z

tap_bing_ads/__init__.py

-        except InvalidDateRangeEnd as ex:
-            LOGGER.warn("Bing reported that the requested report date range ended outside of "
+        except InvalidDateRangeEnd as ex: # pylint: disable=unused-variable
+            LOGGER.warn("Bing reported that the requested report date range ended outside of " # pylint: disable=deprecated-method


Put deprecated-method in config.yml

dbshah1212 · 2021-10-07T10:45:06Z

tap_bing_ads/__init__.py

        singer.write_bookmark(STATE, state_key, 'request_id', None)
        singer.write_state(STATE)
        return False


-def get_report_request_id(client, account_id, report_stream, report_name,
+def get_report_request_id(client, account_id, report_stream, report_name,    # pylint: disable=too-many-arguments


Put too-many-arguments in config.yml and removed this from other places

dbshah1212 · 2021-10-07T10:46:08Z

tap_bing_ads/__init__.py


-    report_request = client.factory.create('{}Request'.format(report_name))
+    report_request = client.factory.create(f'{report_name}Request'.format(report_name)) # pylint: disable=consider-using-f-string


.format is not required if you are using f-stream. Please check the usage of f-stream

tap_bing_ads/__init__.py

umangagrawal-crest · 2021-10-26T12:36:07Z

tap_bing_ads/__init__.py

@@ -102,7 +101,7 @@ def set_options(self, **kwargs):

 def create_sdk_client(service, account_id):
    LOGGER.info('Creating SOAP client with OAuth refresh credentials for service: %s, account_id %s',
-                service, account_id)
+                str(service), account_id)


Why we have added str?

We didn't need that, so removed it

…m/singer-io/tap-bing-ads into TDL-6737-add-pylint-to-tap-tester

umangagrawal-crest · 2021-10-27T13:00:02Z

tap_bing_ads/__init__.py

-                        'Error polling {} for account {} with request id {}'
-                        .format(report_name, account_id, request_id))
+                LOGGER.warn('Error polling %s for account %s with request id %s',
+                            report_name, str(account_id), str(request_id))


Need to remove str from here as well

umangagrawal-crest · 2021-10-27T13:00:27Z

tap_bing_ads/__init__.py

+                LOGGER.info('Generating report timed out: %s - from %s to %s',
+                            report_name,
+                            str(start_date),
+                            str(end_date))


same as above

umangagrawal-crest

@namrata270998 Remove str from the files, if not required

…b.com/singer-io/tap-bing-ads into TDL-6737-add-pylint-to-tap-tester

savan-chovatiya · 2021-12-10T07:35:44Z

tap_bing_ads/__init__.py

@@ -358,7 +357,7 @@ def discover_core_objects():
    return core_object_streams

 def get_report_schema(client, report_name):
-    column_obj_name = '{}Column'.format(report_name)
+    column_obj_name = '{}Column'.format(report_name) # pylint: disable=consider-using-f-string


We can disable consider-using-f-string directly into the config.yml command and remove all the disabled comments in the code because it is present at multiple places.

dbshah1212 · 2021-12-15T07:06:47Z

tap_bing_ads/__init__.py

-        raise Exception('Non-200 ({}) response downloading report: {}'.format(
-            response.status_code, report_name))
-
+        raise Exception('Non-200 ({}) response downloading report: {}'.format(response.status_code, report_name))


Reverte this change if it is not throwing any pylint error.

dbshah1212 · 2021-12-15T07:07:50Z

tap_bing_ads/__init__.py

@@ -864,8 +855,7 @@ def build_report_request(client, account_id, report_stream, report_name,
    report_columns = client.factory.create(
        'ArrayOf{}Column'.format(report_name)
    )
-    getattr(report_columns, '{}Column'.format(report_name)) \
-        .append(selected_fields)
+    getattr(report_columns, '{}Column'.format(report_name)).append(selected_fields)


revert this change if it is not throwing any pylint error.

KrisPersonal · 2021-12-17T16:49:33Z

tap_bing_ads/__init__.py

+        log_args = list(map(lambda arg: str(arg).replace('\n', '\\n'), args)) + list(map(lambda kv: '{}={}'.format(*kv), kwargs.items()))
+        LOGGER.info('Calling: %s(%s) for account: %s',
+                    service_method.name,
+                    ','.join(log_args),


What is the difference between what was before and the changes done?

We have removed format and used %s instead, as pylint was throwing error for format

…b.com/singer-io/tap-bing-ads into TDL-6737-add-pylint-to-tap-tester

* TDL-16888 Added code comments (#88) * Added code comments * Updated code comments * UPdated comment * TDL 7402 Add retry logic for 500 errors (#90) * Initial commit for backoff error handling * Added usage of error handling function * Added detail comment for unittest cases * Added coverage report to artifact * added backoff for Expection(408) * Updated backoff for async method * updated backoof for poll_report * Updated max_time * Updated unittest cases * removed Exception from backoff * Updated backoff * resolved unittest case errro * Updated typo in comments * Added test cases for Exception with error code 408 * Added comment for 408 error * Added comments * Updated unit test cases for 400 error * Updated unit test case for create sdk client * Merged elif condition * Updated unit test cases * Tdl 9721 missing top level breadcrumb for all streams (#91) * TDL-9721: Updated metadata creation method and added top level breadcrum and un commented the breadcrum test case code. * TDL-9721: Added top level breadcrumb. * TDL-9721: Commented the code wich uncommented by mistake and failing tests * TDL-9721: Removed unused changes * TDL-9721: Updated automatic fields for report streams * TDL-9721: Optimized the code of making replication key automatic * TDL-9721: Added Unit tests with coverage * TDL-9721: Added pip install nose * TDL-9721: Updated file name * TDL-9721: Updated config.yml * TDL-9721: Added missing fieldExclusion * TDL-9721: Added code comment * TDL-9721: Updated the primary key of account stream as earlier. * TDL-9721: Updated primary key for account stream in tests to make it same as catalog. * TDL-9721: Updated primary key in catalog for account stream. * TDL-9721: Updated the replication keys and test cases. * TDL-9721: Updated replication key for discover mode only as other test using replication key. * TDL-6737 added pylint and resolved the errors (#87) * added pylint and resolved the errors * added pylint in the setup.py * corrected the pylint command * added package name * replaced dev with test in pip install * resolved the comments * removed f-string * Added covergae report to artifact * removed unittest case report * removed config file changes * revert back config.yml file changes * Added back pylint in circleci * resolved pylint error * resolved comments * removed str * removed logger statements * added consider-using-f-string in config.yml * resolved comments * resolved pylint errors * pylint fixtures Co-authored-by: prijendev <prijen.khokhani@crestdatasys.com> * TDL-15251: Add required fields for age_gender_audience_report (#86) * added required fields for age_gender_audience_report * Added coverage report to artifact * revert back coverage report changes * revert back config.yml file changes * resolved comments * added missing assertions * resolved cci failures Co-authored-by: namrata270998 <namrata.brahmbhatt@crestdatasys.com> * Tdl 7402 add retry logic for 500 errors (#96) * Initial commit for backoff error handling * Added usage of error handling function * Added detail comment for unittest cases * Added coverage report to artifact * added backoff for Expection(408) * Updated backoff for async method * updated backoof for poll_report * Updated max_time * Updated unittest cases * removed Exception from backoff * Updated backoff * resolved unittest case errro * Updated typo in comments * Added test cases for Exception with error code 408 * Added comment for 408 error * Added comments * Updated unit test cases for 400 error * Updated unit test case for create sdk client * Merged elif condition * Updated unit test cases * Tdl 15861 implement request timeout (#93) * TDL-15861 implement request timeout * added omment * removed unused imports * added comment * resolved confilcts with the base * resolved conflicts * resolved conflicts * resolved comments * updated unittests * added timeout for get calls * corrected a typo in comment * resolved comments * resolved comments Co-authored-by: prijendev <prijen.khokhani@crestdatasys.com> Co-authored-by: prijendev <prijen.khokhani@crestdatasys.com> * TDL-9902 removed the automatic fields from ad ext report (#85) * TDL-9902 removed the automatic fields from ad ext report * Added coverage report in artifact * revert back config.yml file * resolved comments Co-authored-by: namrata270998 <namrata.brahmbhatt@crestdatasys.com> Co-authored-by: prijendev <88327452+prijendev@users.noreply.github.com> Co-authored-by: dbshah1212 <35164219+dbshah1212@users.noreply.github.com> Co-authored-by: prijendev <prijen.khokhani@crestdatasys.com>

namrata270998 and others added 5 commits October 7, 2021 14:12

added pylint and resolved the errors

1b1fcf6

added pylint in the setup.py

7fcd83f

corrected the pylint command

9251bdc

added package name

cb15949

replaced dev with test in pip install

0857924

dbshah1212 reviewed Oct 7, 2021

View reviewed changes

tap_bing_ads/__init__.py Outdated Show resolved Hide resolved

resolved the comments

d1b1482

dbshah1212 self-requested a review October 14, 2021 07:39

singer-io deleted a comment from namrata270998 Oct 14, 2021

dbshah1212 approved these changes Oct 14, 2021

View reviewed changes

namrata270998 and others added 7 commits October 18, 2021 16:15

removed f-string

77cbfd6

Added covergae report to artifact

2d3d110

removed unittest case report

ebe6c38

removed config file changes

d69a23a

revert back config.yml file changes

0523a2f

Added back pylint in circleci

a448d21

resolved pylint error

b752958

umangagrawal-crest reviewed Oct 26, 2021

View reviewed changes

namrata270998 added 2 commits October 27, 2021 18:22

resolved comments

c4f87b9

Merge branch 'TDL-6737-add-pylint-to-tap-tester' of https://github.co…

ea74d52

…m/singer-io/tap-bing-ads into TDL-6737-add-pylint-to-tap-tester

umangagrawal-crest reviewed Oct 27, 2021

View reviewed changes

umangagrawal-crest suggested changes Oct 27, 2021

View reviewed changes

namrata270998 added 2 commits October 27, 2021 20:03

removed str

27cc866

removed logger statements

63a813d

namrata270998 requested a review from umangagrawal-crest October 28, 2021 04:33

Merge branch 'TDL-10048-Improve-BingAds-Reliability' of https://githu…

f3ff26a

…b.com/singer-io/tap-bing-ads into TDL-6737-add-pylint-to-tap-tester

savan-chovatiya reviewed Dec 10, 2021

View reviewed changes

added consider-using-f-string in config.yml

77a0eea

dbshah1212 requested a review from savan-chovatiya December 15, 2021 07:04

dbshah1212 reviewed Dec 15, 2021

View reviewed changes

resolved comments

a59b032

namrata270998 requested a review from dbshah1212 December 15, 2021 08:38

karanpanchal-crest approved these changes Dec 16, 2021

View reviewed changes

namrata270998 changed the title ~~added pylint and resolved the errors~~ TDL-6737 added pylint and resolved the errors Dec 17, 2021

namrata270998 requested review from KrisPersonal, manand31 and kspeer825 December 17, 2021 07:47

umangagrawal-crest approved these changes Dec 17, 2021

View reviewed changes

savan-chovatiya approved these changes Dec 17, 2021

View reviewed changes

KrisPersonal reviewed Dec 17, 2021

View reviewed changes

namrata270998 requested a review from KrisPersonal December 20, 2021 06:09

kspeer825 approved these changes Dec 20, 2021

View reviewed changes

prijendev added 3 commits December 22, 2021 15:43

Merge branch 'TDL-10048-Improve-BingAds-Reliability' of https://githu…

e8c58ff

…b.com/singer-io/tap-bing-ads into TDL-6737-add-pylint-to-tap-tester

resolved pylint errors

5b9d313

pylint fixtures

fd066e9

namrata270998 merged commit 9791d73 into TDL-10048-Improve-BingAds-Reliability Dec 22, 2021

namrata270998 mentioned this pull request Dec 23, 2021

Tdl 10048 improve bing ads reliability #97

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TDL-6737 added pylint and resolved the errors #87

TDL-6737 added pylint and resolved the errors #87

namrata270998 commented Oct 7, 2021

dbshah1212 Oct 7, 2021

namrata270998 Oct 7, 2021

dbshah1212 Oct 7, 2021

namrata270998 Oct 7, 2021

dbshah1212 Oct 7, 2021

namrata270998 Oct 7, 2021

umangagrawal-crest Oct 26, 2021

namrata270998 Oct 27, 2021

umangagrawal-crest Oct 27, 2021

namrata270998 Oct 27, 2021

umangagrawal-crest Oct 27, 2021

namrata270998 Oct 27, 2021

umangagrawal-crest left a comment

savan-chovatiya Dec 10, 2021

namrata270998 Dec 10, 2021

dbshah1212 Dec 15, 2021

namrata270998 Dec 15, 2021

dbshah1212 Dec 15, 2021

namrata270998 Dec 15, 2021

KrisPersonal Dec 17, 2021

namrata270998 Dec 20, 2021


		report_request = client.factory.create('{}Request'.format(report_name))
		report_request = client.factory.create(f'{report_name}Request'.format(report_name)) # pylint: disable=consider-using-f-string

TDL-6737 added pylint and resolved the errors #87

TDL-6737 added pylint and resolved the errors #87

Conversation

namrata270998 commented Oct 7, 2021

Description of change

Manual QA steps

Risks

Rollback steps

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

umangagrawal-crest left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment