Merge pull request #40 from dataiku/fix/sc-94528-handling-html-error-…

…messages

Handling HTML error messages

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## [Version 1.0.12](https://github.com/dataiku/dss-plugin-sharepoint-online/releases/tag/v1.0.12) - Feature and bugfix release - 2022-07-19
+
+- Add site path overwrite for username / password permissions presets
+- Handles non JSON error pages
+- Fixes possible hang on read operations
+
 ## [Version 1.0.11](https://github.com/dataiku/dss-plugin-sharepoint-online/releases/tag/v1.0.11) - Feature and bugfix release - 2022-06-21
 
 - Add site path overwrite for site app permissions presets

plugin.json

@@ -1,6 +1,6 @@
 {
     "id": "sharepoint-online",
-    "version": "1.0.11",
+    "version": "1.0.12",
     "meta": {
         "label": "SharePoint Online",
         "description": "Read and write data from/to your SharePoint Online account",

python-connectors/sharepoint-online_lists/connector.py

@@ -1,23 +1,22 @@
 from dataiku.connector import Connector
-import logging
 
 from sharepoint_client import SharePointClient
 from sharepoint_constants import SharePointConstants
 from sharepoint_lists import assert_list_title, get_dss_type
 from sharepoint_lists import SharePointListWriter, column_ids_to_names, sharepoint_to_dss_date
 from common import parse_query_string_to_dict
+from safe_logger import SafeLogger
+from dss_constants import DSSConstants
 
 
-logger = logging.getLogger(__name__)
-logging.basicConfig(level=logging.INFO,
-                    format='sharepoint-online plugin %(levelname)s - %(message)s')
+logger = SafeLogger("sharepoint-online plugin", DSSConstants.SECRET_PARAMETERS_KEYS)
 
 
 class SharePointListsConnector(Connector):
 
     def __init__(self, config, plugin_config):
         Connector.__init__(self, config, plugin_config)
-        logger.info('SharePoint Online plugin connector v1.0.11')
+        logger.info('SharePoint Online plugin connector v1.0.12')
         self.sharepoint_list_title = self.config.get("sharepoint_list_title")
         self.auth_type = config.get('auth_type')
         logger.info('init:sharepoint_list_title={}, auth_type={}'.format(self.sharepoint_list_title, self.auth_type))

python-fs-providers/sharepoint-online_shared-documents/fs-provider.py

@@ -2,22 +2,20 @@
 
 import os
 import shutil
-import logging
 
 from sharepoint_client import SharePointClient
 from dss_constants import DSSConstants
 from sharepoint_items import loop_sharepoint_items, has_sharepoint_items, extract_item_from, get_size, get_last_modified, get_name, assert_path_is_not_root
 from sharepoint_items import create_path
 from common import get_rel_path, get_lnt_path
+from safe_logger import SafeLogger
 
 try:
     from BytesIO import BytesIO  # for Python 2
 except ImportError:
     from io import BytesIO  # for Python 3
 
-logger = logging.getLogger(__name__)
-logging.basicConfig(level=logging.INFO,
-                    format='sharepoint-online plugin %(levelname)s - %(message)s')
+logger = SafeLogger("sharepoint-online plugin", DSSConstants.SECRET_PARAMETERS_KEYS)
 
 
 # based on https://docs.microsoft.com/fr-fr/sharepoint/dev/sp-add-ins/working-with-folders-and-files-with-rest
@@ -33,7 +31,7 @@ def __init__(self, root, config, plugin_config):
             root = root[1:]
         self.root = root
         self.provider_root = "/"
-        logger.info('SharePoint Online plugin fs v1.0.11')
+        logger.info('SharePoint Online plugin fs v1.0.12')
         logger.info('init:root={}'.format(self.root))
 
         self.client = SharePointClient(config)

python-lib/common.py

@@ -7,8 +7,9 @@
     import urllib.parse as urlparse
 from safe_logger import SafeLogger
 from sharepoint_constants import SharePointConstants
+from dss_constants import DSSConstants
 
-logger = SafeLogger("sharepoint-online plugin", ["Authorization", "sharepoint_username", "sharepoint_password", "client_secret"])
+logger = SafeLogger("sharepoint-online plugin", DSSConstants.SECRET_PARAMETERS_KEYS)
 
 
 def get_rel_path(path):
@@ -68,6 +69,9 @@ def is_request_performed(response):
         return False
     if response.status_code in [429, 503]:
         logger.warning("Error {}, headers = {}".format(response.status_code, response.headers))
+        if response.status_code == 503:
+            # SP 503 errors tend to generate html error message, so we dump it in the logs
+            logger.warning("dumping content: {}".format(response.content))
         seconds_before_retry = decode_retry_after_header(response)
         logger.warning("Sleeping for {} seconds".format(seconds_before_retry))
         time.sleep(seconds_before_retry)

python-lib/dss_constants.py

@@ -28,6 +28,7 @@ class DSSConstants(object):
         "sharepoint_oauth": "The access token is missing"
     }
     PATH = 'path'
+    SECRET_PARAMETERS_KEYS = ["Authorization", "sharepoint_username", "sharepoint_password", "client_secret"]
     SITE_APP_DETAILS = {
         "sharepoint_tenant": "The tenant name is missing",
         "sharepoint_site": "The site name is missing",

python-lib/robust_session.py

@@ -1,10 +1,9 @@
-import logging
 import time
+from safe_logger import SafeLogger
+from dss_constants import DSSConstants
 
 
-logger = logging.getLogger(__name__)
-logging.basicConfig(level=logging.INFO,
-                    format='sharepoint-online plugin %(levelname)s - %(message)s')
+logger = SafeLogger("sharepoint-online plugin", DSSConstants.SECRET_PARAMETERS_KEYS)
 
 
 class RobustSessionError(ValueError):

python-lib/sharepoint_client.py

@@ -17,7 +17,7 @@
 from safe_logger import SafeLogger
 
 
-logger = SafeLogger("sharepoint-online plugin", ["Authorization", "sharepoint_username", "sharepoint_password", "client_secret"])
+logger = SafeLogger("sharepoint-online plugin", DSSConstants.SECRET_PARAMETERS_KEYS)
 
 
 class SharePointClientError(ValueError):
@@ -31,7 +31,7 @@ def __init__(self, config):
         self.sharepoint_url = None
         self.sharepoint_origin = None
         attempt_session_reset_on_403 = config.get("advanced_parameters", False) and config.get("attempt_session_reset_on_403", False)
-        self.session = RobustSession(status_codes_to_retry=[429], attempt_session_reset_on_403=attempt_session_reset_on_403)
+        self.session = RobustSession(status_codes_to_retry=[429, 503], attempt_session_reset_on_403=attempt_session_reset_on_403)
         self.number_dumped_logs = 0
         self.username_for_namespace_diag = None
         if config.get('auth_type') == DSSConstants.AUTH_OAUTH:
@@ -57,6 +57,7 @@ def __init__(self, config):
             login_details = config.get('sharepoint_sharepy')
             self.assert_login_details(DSSConstants.LOGIN_DETAILS, login_details)
             self.setup_login_details(login_details)
+            self.apply_paths_overwrite(config)
             username = login_details['sharepoint_username']
             password = login_details['sharepoint_password']
             self.assert_email_address(username)
@@ -694,6 +695,8 @@ def assert_response_ok(self, response, no_json=False, calling_method=""):
         status_code = response.status_code
         if status_code >= 400:
             logger.error("Error {} in method {}".format(status_code, calling_method))
+            logger.error("when calling {}".format(response.url))
+            logger.error("dump={}".format(response.content))
             enriched_error_message = self.get_enriched_error_message(response)
             if enriched_error_message is not None:
                 raise SharePointClientError("Error ({}): {}".format(calling_method, enriched_error_message))
@@ -780,7 +783,8 @@ def get_site_app_access_token(self):
         response = requests.post(
             SharePointConstants.GET_SITE_APP_TOKEN_URL.format(tenant_id=self.tenant_id),
             headers=headers,
-            data=data
+            data=data,
+            timeout=SharePointConstants.TIMEOUT_SEC
         )
         self.assert_response_ok(response, calling_method="get_site_app_access_token")
         json_response = response.json()
@@ -824,7 +828,7 @@ def get(self, url, headers=None, params=None):
         headers["Authorization"] = self.get_authorization_bearer()
         response = None
         while not is_request_performed(response) and not retries_limit.is_reached():
-            response = requests.get(url, headers=headers, params=params)
+            response = requests.get(url, headers=headers, params=params, timeout=SharePointConstants.TIMEOUT_SEC)
         return response
 
     def post(self, url, headers=None, json=None, data=None, params=None):
@@ -852,15 +856,18 @@ def get_authorization_bearer(self):
 
     def get_form_digest_value(self):
         logger.info("Getting form digest value")
-        session = RobustSession(session=requests, status_codes_to_retry=[429])
+        session = RobustSession(session=requests, status_codes_to_retry=[429, 503])
         session.update_settings(
             max_retries=SharePointConstants.MAX_RETRIES,
             base_retry_timer_sec=SharePointConstants.WAIT_TIME_BEFORE_RETRY_SEC
         )
         headers = {**DSSConstants.JSON_HEADERS, **{"Authorization": self.get_authorization_bearer()}}
+
+        # Treat this as a response = requests.post(
         response = session.post(
             url=self.get_contextinfo_url(),
-            headers=headers
+            headers=headers,
+            timeout=SharePointConstants.TIMEOUT_SEC
         )
         form_digest_value = get_value_from_path(
             response.json(),

python-lib/sharepoint_lists.py

@@ -1,13 +1,11 @@
-import logging
 import datetime
 from concurrent.futures import ThreadPoolExecutor, as_completed
 from sharepoint_constants import SharePointConstants
 from dss_constants import DSSConstants
+from safe_logger import SafeLogger
 
 
-logger = logging.getLogger(__name__)
-logging.basicConfig(level=logging.INFO,
-                    format='sharepoint plugin %(levelname)s - %(message)s')
+logger = SafeLogger("sharepoint-online plugin", DSSConstants.SECRET_PARAMETERS_KEYS)
 
 
 def is_response_empty(response):

tests/python/unit/test_common.py

@@ -7,6 +7,8 @@ class MockResponse:
     def __init__(self, status_code, headers):
         self.status_code = status_code
         self.headers = headers
+        self.content = '{"a": 1}'
+        self.url = 'https://test.com/test'
 
 
 class TestCommonMethods: