From 3262fdf3fcd2503ec720f0f329621e56741f8776 Mon Sep 17 00:00:00 2001 From: Ashwin Thanaraj <37061471+ashwin1111@users.noreply.github.com> Date: Wed, 18 Jan 2023 13:22:42 +0530 Subject: [PATCH] Updating logging levels (#201) * Updating logging levels * fix lint * rm workflow * ignore exception * more exception * few more fixes * fix pylint * fix test * add more exception * remove 4xx logger * remove 4xx logger * add logging level * rm logger * rm --- .github/workflows/pylint.yml | 18 ----- apps/fyle/models.py | 2 +- apps/mappings/tasks.py | 106 ++++++++++++++++++---------- apps/mappings/views.py | 9 ++- apps/workspaces/views.py | 11 +-- apps/xero/tasks.py | 71 ++++++++++++------- apps/xero/utils.py | 12 ++-- apps/xero/views.py | 15 ++++ fyle_xero_api/logging_middleware.py | 6 +- tests/test_mappings/test_tasks.py | 6 +- 10 files changed, 150 insertions(+), 106 deletions(-) delete mode 100644 .github/workflows/pylint.yml diff --git a/.github/workflows/pylint.yml b/.github/workflows/pylint.yml deleted file mode 100644 index b1971c1f..00000000 --- a/.github/workflows/pylint.yml +++ /dev/null @@ -1,18 +0,0 @@ -name: Pylint Check - -on: - pull_request: - types: [assigned, opened, synchronize, reopened] - -jobs: - build: - - runs-on: ubuntu-latest - - steps: - - uses: actions/checkout@v1 - - name: Python Pylin GitHub Action - uses: fylein/python-pylint-github-action@v4 - with: - args: pip3 install -r requirements.txt && pip install pylint-django==2.3.0 && pylint --load-plugins pylint_django --rcfile=.pylintrc **/**.py - diff --git a/apps/fyle/models.py b/apps/fyle/models.py index 51ac2d8d..e8a9d737 100644 --- a/apps/fyle/models.py +++ b/apps/fyle/models.py @@ -156,7 +156,7 @@ def create_expense_objects(expenses: List[Dict], workspace_id: int): eliminated_expenses.append(expense['id']) if eliminated_expenses: - logger.error('Expenses with ids {} are not eligible for import'.format(eliminated_expenses)) + logger.info('Expenses with ids {} are not eligible for import'.format(eliminated_expenses)) return expense_objects diff --git a/apps/mappings/tasks.py b/apps/mappings/tasks.py index 7f9eddb1..38982686 100644 --- a/apps/mappings/tasks.py +++ b/apps/mappings/tasks.py @@ -12,6 +12,8 @@ from fyle.platform.exceptions import WrongParamsError +from xerosdk.exceptions import UnsuccessfulAuthentication, InvalidGrant + from apps.xero.utils import XeroConnector from apps.tasks.models import Error from apps.workspaces.models import XeroCredentials, FyleCredential, WorkspaceGeneralSettings @@ -111,43 +113,35 @@ def upload_categories_to_fyle(workspace_id): """ Upload categories to Fyle """ - try: - fyle_credentials: FyleCredential = FyleCredential.objects.get(workspace_id=workspace_id) - xero_credentials: XeroCredentials = XeroCredentials.get_active_xero_credentials(workspace_id) - general_settings = WorkspaceGeneralSettings.objects.filter(workspace_id=workspace_id).first() - platform = PlatformConnector(fyle_credentials) - - category_map = get_all_categories_from_fyle(platform=platform) - - xero_connection = XeroConnector( - credentials_object=xero_credentials, - workspace_id=workspace_id - ) - platform.categories.sync() - xero_connection.sync_accounts() + fyle_credentials: FyleCredential = FyleCredential.objects.get(workspace_id=workspace_id) + xero_credentials: XeroCredentials = XeroCredentials.get_active_xero_credentials(workspace_id) + general_settings = WorkspaceGeneralSettings.objects.filter(workspace_id=workspace_id).first() + platform = PlatformConnector(fyle_credentials) - xero_attributes = DestinationAttribute.objects.filter( - workspace_id=workspace_id, - attribute_type='ACCOUNT', - detail__account_type__in=general_settings.charts_of_accounts - ).all() + category_map = get_all_categories_from_fyle(platform=platform) - xero_attributes = remove_duplicates(xero_attributes) + xero_connection = XeroConnector( + credentials_object=xero_credentials, + workspace_id=workspace_id + ) + platform.categories.sync() + xero_connection.sync_accounts() - fyle_payload: List[Dict] = create_fyle_categories_payload(xero_attributes, workspace_id, category_map) + xero_attributes = DestinationAttribute.objects.filter( + workspace_id=workspace_id, + attribute_type='ACCOUNT', + detail__account_type__in=general_settings.charts_of_accounts + ).all() - if fyle_payload: - platform.categories.post_bulk(fyle_payload) - platform.categories.sync() + xero_attributes = remove_duplicates(xero_attributes) - return xero_attributes + fyle_payload: List[Dict] = create_fyle_categories_payload(xero_attributes, workspace_id, category_map) - except XeroCredentials.DoesNotExist: - logger.error( - 'Xero Credentials not found for workspace_id %s', - workspace_id, - ) + if fyle_payload: + platform.categories.post_bulk(fyle_payload) + platform.categories.sync() + return xero_attributes def auto_create_category_mappings(workspace_id): """ @@ -164,6 +158,15 @@ def auto_create_category_mappings(workspace_id): return [] + except XeroCredentials.DoesNotExist: + logger.info( + 'Xero Credentials not found for workspace_id %s', + workspace_id, + ) + + except (UnsuccessfulAuthentication, InvalidGrant): + logger.info('Xero refresh token is invalid for workspace_id - %s', workspace_id) + except WrongParamsError as exception: logger.error( 'Error while creating categories workspace_id - %s in Fyle %s %s', @@ -220,11 +223,13 @@ def async_auto_map_employees(workspace_id: int): resolve_expense_attribute_errors(source_attribute_type='EMPLOYEE', workspace_id=workspace_id) except XeroCredentials.DoesNotExist: - logger.error( + logger.info( 'Xero Credentials not found for workspace_id %s', workspace_id, ) + except (UnsuccessfulAuthentication, InvalidGrant): + logger.info('Xero refresh token is invalid for workspace_id - %s', workspace_id) def schedule_auto_map_employees(employee_mapping_preference: str, workspace_id: str): if employee_mapping_preference: @@ -334,12 +339,18 @@ def auto_create_cost_center_mappings(workspace_id: int): post_cost_centers_in_batches(platform, workspace_id, mapping_setting.destination_field) + except XeroCredentials.DoesNotExist: + logger.info('Xero credentials does not exist for workspace_id - %s', workspace_id) + except WrongParamsError as exception: logger.error( 'Error while creating cost centers workspace_id - %s in Fyle %s %s', workspace_id, exception.message, {'error': exception.response} ) + except (UnsuccessfulAuthentication, InvalidGrant): + logger.info('Xero refresh token is invalid for workspace_id - %s', workspace_id) + except Exception: error = traceback.format_exc() error = { @@ -440,12 +451,18 @@ def auto_create_project_mappings(workspace_id: int): post_projects_in_batches(platform, workspace_id, mapping_setting.destination_field) + except XeroCredentials.DoesNotExist: + logger.info('Xero credentials does not exist for workspace_id - %s', workspace_id) + except WrongParamsError as exception: logger.error( 'Error while creating projects workspace_id - %s in Fyle %s %s', workspace_id, exception.message, {'error': exception.response} ) + except (UnsuccessfulAuthentication, InvalidGrant): + logger.info('Xero refresh token is invalid for workspace_id - %s', workspace_id) + except Exception: error = traceback.format_exc() error = { @@ -603,12 +620,19 @@ def async_auto_create_custom_field_mappings(workspace_id: str): ).all() for mapping_setting in mapping_settings: - if mapping_setting.import_to_fyle: - sync_xero_attributes(mapping_setting.destination_field, workspace_id) - auto_create_expense_fields_mappings( - workspace_id, mapping_setting.destination_field, mapping_setting.source_field, - mapping_setting.source_placeholder - ) + try: + if mapping_setting.import_to_fyle: + sync_xero_attributes(mapping_setting.destination_field, workspace_id) + auto_create_expense_fields_mappings( + workspace_id, mapping_setting.destination_field, mapping_setting.source_field, + mapping_setting.source_placeholder + ) + + except XeroCredentials.DoesNotExist: + logger.info('Xero credentials does not exist for workspace_id - %s', workspace_id) + + except (UnsuccessfulAuthentication, InvalidGrant): + logger.info('Xero refresh token is invalid for workspace_id - %s', workspace_id) def schedule_fyle_attributes_creation(workspace_id: int): @@ -691,18 +715,24 @@ def auto_create_tax_codes_mappings(workspace_id: int): upload_tax_groups_to_fyle(platform, workspace_id) + except XeroCredentials.DoesNotExist: + logger.info('Xero credentials does not exist for workspace_id - %s', workspace_id) + except WrongParamsError as exception: logger.error( 'Error while creating tax groups workspace_id - %s in Fyle %s %s', workspace_id, exception.message, {'error': exception.response} ) + except (UnsuccessfulAuthentication, InvalidGrant): + logger.info('Xero refresh token is invalid for workspace_id - %s', workspace_id) + except Exception: error = traceback.format_exc() error = { 'error': error } - logger.error( + logger.exception( 'Error while creating tax groups workspace_id - %s error: %s', workspace_id, error ) diff --git a/apps/mappings/views.py b/apps/mappings/views.py index d8ab8de1..407d7055 100644 --- a/apps/mappings/views.py +++ b/apps/mappings/views.py @@ -7,6 +7,8 @@ from fyle_xero_api.utils import assert_valid +from xerosdk.exceptions import UnsuccessfulAuthentication + from .serializers import TenantMappingSerializer, GeneralMappingSerializer from .models import TenantMapping, GeneralMapping from apps.workspaces.models import XeroCredentials @@ -54,8 +56,11 @@ def post(self, request, *args, **kwargs): tenant_mapping.connection_id = connection[0]['id'] tenant_mapping.save() - except: - logger.error('Error while fetching company information') + except UnsuccessfulAuthentication: + logger.info('Xero refresh token is invalid for workspace_id - %s', kwargs['workspace_id']) + + except Exception: + logger.info('Error while fetching company information') return Response( data=self.serializer_class(tenant_mapping_object).data, diff --git a/apps/workspaces/views.py b/apps/workspaces/views.py index 5349170f..471a6c57 100644 --- a/apps/workspaces/views.py +++ b/apps/workspaces/views.py @@ -308,8 +308,8 @@ def post(self, request, **kwargs): xero_credentials.country = company_info['CountryCode'] xero_credentials.save() - except xero_exc.WrongParamsError as exception: - logger.error(exception.response) + except (xero_exc.WrongParamsError, xero_exc.UnsuccessfulAuthentication) as exception: + logger.info(exception.response) if workspace.onboarding_state == 'CONNECTION': workspace.onboarding_state = 'EXPORT_SETTINGS' @@ -319,12 +319,7 @@ def post(self, request, **kwargs): data=XeroCredentialSerializer(xero_credentials).data, status=status.HTTP_200_OK ) - except xero_exc.InvalidClientError as e: - return Response( - json.loads(e.response), - status=status.HTTP_400_BAD_REQUEST - ) - except xero_exc.InvalidGrant as e: + except (xero_exc.InvalidClientError, xero_exc.InvalidGrant, xero_exc.UnsuccessfulAuthentication) as e: return Response( json.loads(e.response), status=status.HTTP_400_BAD_REQUEST diff --git a/apps/xero/tasks.py b/apps/xero/tasks.py index c740634b..24ec062c 100644 --- a/apps/xero/tasks.py +++ b/apps/xero/tasks.py @@ -13,7 +13,7 @@ from fyle_accounting_mappings.models import Mapping, ExpenseAttribute, DestinationAttribute from fyle_integrations_platform_connector import PlatformConnector -from xerosdk.exceptions import XeroSDKError, WrongParamsError, InvalidGrant, RateLimitError, NoPrivilegeError +from xerosdk.exceptions import XeroSDKError, WrongParamsError, InvalidGrant, RateLimitError, NoPrivilegeError, UnsuccessfulAuthentication from apps.fyle.models import ExpenseGroup, Reimbursement, Expense from apps.tasks.models import Error @@ -57,7 +57,7 @@ def handle_xero_error(exception, expense_group: ExpenseGroup, task_log: TaskLog) :params export_type: export_type """ if type(exception).__name__ == 'RateLimitError': - logger.error(exception.message) + logger.info(exception.message) task_log.xero_errors = [ { 'error': { @@ -86,7 +86,7 @@ def handle_xero_error(exception, expense_group: ExpenseGroup, task_log: TaskLog) else: all_details = [] - logger.exception(exception) + logger.info(exception) detail = json.dumps(exception.__dict__) detail = json.loads(detail) @@ -147,7 +147,7 @@ def get_or_create_credit_card_contact(workspace_id: int, merchant: str, auto_cre try: contact = xero_connection.get_or_create_contact(merchant, create=False) except WrongParamsError as bad_request: - logger.error(bad_request.message) + logger.info(bad_request.message) if not contact and auto_create_merchant_destination_entity: merchant = merchant @@ -159,7 +159,7 @@ def get_or_create_credit_card_contact(workspace_id: int, merchant: str, auto_cre try: contact = xero_connection.get_or_create_contact(merchant, create=True) except WrongParamsError as bad_request: - logger.error(bad_request.message) + logger.info(bad_request.message) return contact @@ -339,7 +339,7 @@ def create_bill(expense_group_id: int, task_log_id: int, xero_connection: XeroCo handle_xero_error(exception=exception, expense_group=expense_group, task_log=task_log) except InvalidGrant as exception: - logger.exception(exception.message) + logger.info(exception.message) task_log.status = 'FAILED' task_log.detail = None task_log.xero_errors = { @@ -357,7 +357,7 @@ def create_bill(expense_group_id: int, task_log_id: int, xero_connection: XeroCo xero_credentials.country = None xero_credentials.is_expired = True xero_credentials.save() - logger.error(exception.message) + logger.info(exception.message) task_log.status = 'FAILED' task_log.detail = None task_log.xero_errors = [ @@ -379,7 +379,7 @@ def create_bill(expense_group_id: int, task_log_id: int, xero_connection: XeroCo task_log.save() except XeroSDKError as exception: - logger.exception(exception.response) + logger.info(exception.response) detail = exception.response task_log.status = 'FAILED' task_log.detail = None @@ -408,21 +408,24 @@ def create_chain_and_export(chaining_attributes: list, workspace_id: int) -> Non :param workspace_id: :return: None """ - xero_credentials = XeroCredentials.get_active_xero_credentials(workspace_id) - xero_connection = XeroConnector(xero_credentials, workspace_id) + try: + xero_credentials = XeroCredentials.get_active_xero_credentials(workspace_id) + xero_connection = XeroConnector(xero_credentials, workspace_id) - chain = Chain() + chain = Chain() - fyle_credentials = FyleCredential.objects.get(workspace_id=workspace_id) - chain.append('apps.fyle.tasks.sync_dimensions', fyle_credentials) + fyle_credentials = FyleCredential.objects.get(workspace_id=workspace_id) + chain.append('apps.fyle.tasks.sync_dimensions', fyle_credentials) - for group in chaining_attributes: - trigger_function = 'apps.xero.tasks.create_{}'.format(group['export_type']) - chain.append(trigger_function, group['expense_group_id'], group['task_log_id'], xero_connection, group['last_export']) + for group in chaining_attributes: + trigger_function = 'apps.xero.tasks.create_{}'.format(group['export_type']) + chain.append(trigger_function, group['expense_group_id'], group['task_log_id'], xero_connection, group['last_export']) - if chain.length() > 1: - chain.run() + if chain.length() > 1: + chain.run() + except UnsuccessfulAuthentication: + logger.info('Xero refresh token is invalid for workspace_id - %s', workspace_id) def schedule_bills_creation(workspace_id: int, expense_group_ids: List[str]) -> list: """ @@ -527,7 +530,7 @@ def attach_customer_to_export(xero_connection: XeroConnector, task_log: TaskLog) except Exception as exception: # Silently ignoring the error, since the export should be already created - logger.exception('Something unexpected happened during attaching customer to export', exception) + logger.info('Something unexpected happened during attaching customer to export', exception) def create_bank_transaction(expense_group_id: int, task_log_id: int, xero_connection: XeroConnector, last_export: bool): @@ -620,7 +623,7 @@ def create_bank_transaction(expense_group_id: int, task_log_id: int, xero_connec handle_xero_error(exception=exception, expense_group=expense_group, task_log=task_log) except InvalidGrant as exception: - logger.exception(exception.message) + logger.info(exception.message) task_log.status = 'FAILED' task_log.detail = None task_log.xero_errors = { @@ -632,13 +635,13 @@ def create_bank_transaction(expense_group_id: int, task_log_id: int, xero_connec except RateLimitError as exception: handle_xero_error(exception=exception, expense_group=expense_group, task_log=task_log) - except NoPrivilegeError as exception: + except (NoPrivilegeError, UnsuccessfulAuthentication) as exception: xero_credentials = XeroCredentials.objects.filter(workspace_id=expense_group.workspace_id).first() xero_credentials.refresh_token = None xero_credentials.country = None xero_credentials.is_expired = True xero_credentials.save() - logger.error(exception.message) + logger.info(exception.message) task_log.status = 'FAILED' task_log.detail = None task_log.xero_errors = [ @@ -660,7 +663,7 @@ def create_bank_transaction(expense_group_id: int, task_log_id: int, xero_connec task_log.save() except XeroSDKError as exception: - logger.exception(exception.response) + logger.info(exception.response) detail = exception.response task_log.status = 'FAILED' task_log.detail = None @@ -940,13 +943,13 @@ def create_payment(workspace_id): task_log.save() - except NoPrivilegeError as exception: + except (NoPrivilegeError, UnsuccessfulAuthentication) as exception: xero_credentials = XeroCredentials.objects.filter(workspace_id=workspace_id).first() xero_credentials.refresh_token = None xero_credentials.country = None xero_credentials.is_expired = True xero_credentials.save() - logger.error(exception.message) + logger.info(exception.message) task_log.status = 'FAILED' task_log.detail = None task_log.xero_errors = [ @@ -1053,6 +1056,12 @@ def check_xero_object_status(workspace_id): workspace_id, ) + except UnsuccessfulAuthentication: + logger.info( + 'Xero refresh token expired for workspace_id %s', + workspace_id, + ) + def schedule_xero_objects_status_sync(sync_xero_to_fyle_payments, workspace_id): if sync_xero_to_fyle_payments: @@ -1158,6 +1167,12 @@ def create_missing_currency(workspace_id: int): }) logger.info('Created missing currency %s in Xero', fyle_currency) + except UnsuccessfulAuthentication: + logger.info( + 'Xero refresh token expired for workspace_id %s', + workspace_id, + ) + except Exception as exception: logger.exception('Error creating currency in Xero', exception) @@ -1182,5 +1197,11 @@ def update_xero_short_code(workspace_id: int): logger.info('Updated Xero short code') + except UnsuccessfulAuthentication: + logger.info( + 'Xero refresh token expired for workspace_id %s', + workspace_id, + ) + except Exception as exception: logger.exception('Error updating Xero short code', exception) diff --git a/apps/xero/utils.py b/apps/xero/utils.py index 60b52627..adcde8bf 100644 --- a/apps/xero/utils.py +++ b/apps/xero/utils.py @@ -363,32 +363,32 @@ def sync_dimensions(self, workspace_id: str): try: self.sync_accounts() except Exception as exception: - logger.exception(exception) + logger.info(exception) try: self.sync_contacts() except Exception as exception: - logger.exception(exception) + logger.info(exception) try: self.sync_customers() except Exception as exception: - logger.exception(exception) + logger.info(exception) try: self.sync_items() except Exception as exception: - logger.exception(exception) + logger.info(exception) try: self.sync_tracking_categories() except Exception as exception: - logger.exception(exception) + logger.info(exception) try: self.sync_tax_codes() except Exception as exception: - logger.exception(exception) + logger.info(exception) def post_contact(self, contact_name: str, email: str): """ diff --git a/apps/xero/views.py b/apps/xero/views.py index 4175c759..94f89e50 100644 --- a/apps/xero/views.py +++ b/apps/xero/views.py @@ -327,6 +327,14 @@ def post(self, request, *args, **kwargs): status=status.HTTP_400_BAD_REQUEST ) + except (InvalidGrant, UnsuccessfulAuthentication, InvalidTokenError): + return Response( + data={ + 'message': 'Xero Credentials are invalid' + }, + status=status.HTTP_400_BAD_REQUEST + ) + class RefreshXeroDimensionView(generics.ListCreateAPIView): """ @@ -358,6 +366,13 @@ def post(self, request, *args, **kwargs): status=status.HTTP_400_BAD_REQUEST ) + except (InvalidGrant, UnsuccessfulAuthentication, InvalidTokenError): + return Response( + data={ + 'message': 'Xero credentials are invalid' + }, + status=status.HTTP_400_BAD_REQUEST + ) class PaymentView(generics.CreateAPIView): """ diff --git a/fyle_xero_api/logging_middleware.py b/fyle_xero_api/logging_middleware.py index 16a61b36..1e9c7812 100644 --- a/fyle_xero_api/logging_middleware.py +++ b/fyle_xero_api/logging_middleware.py @@ -13,11 +13,7 @@ def __init__(self, get_response): self.get_response = get_response def __call__(self, request): - response = self.get_response(request) - if response.status_code >= 400: - if 'data' in response.__dict__: - logger.error('%s %s', request.build_absolute_uri(), str(response.data).replace('\n', '')) - return response + return self.get_response(request) def process_exception(self, request, exception): if not settings.DEBUG: diff --git a/tests/test_mappings/test_tasks.py b/tests/test_mappings/test_tasks.py index b154a09b..00684ccc 100644 --- a/tests/test_mappings/test_tasks.py +++ b/tests/test_mappings/test_tasks.py @@ -175,9 +175,9 @@ def test_upload_categories_to_fyle(db, mocker): xero_credentials = XeroCredentials.objects.get(workspace_id=workspace_id) xero_credentials.delete() - xero_attributes = upload_categories_to_fyle(workspace_id=workspace_id) - assert xero_attributes == None - + # Expect XeroCredentials.DoesNotExist exception since we've deleted the credentials + with pytest.raises(XeroCredentials.DoesNotExist): + xero_attributes = upload_categories_to_fyle(workspace_id=workspace_id) def test_create_fyle_category_payload(mocker, db): workspace_id = 1