pabloem commented on a change in pull request #12331: URL: https://github.com/apache/beam/pull/12331#discussion_r463788480
########## File path: sdks/python/apache_beam/io/gcp/dicomio.py ########## @@ -167,33 +167,40 @@ class DicomSearch(PTransform): } """ - def __init__(self, buffer_size=8, max_workers=5, credential=None): + def __init__( + self, buffer_size=8, max_workers=5, client=None, credential=None): """Initializes DicomSearch. Args: credential: # type: Google credential object, if it is specified, the Http client will use it to create sessions instead of the default. """ self.buffer_size = buffer_size self.max_workers = max_workers + if not client: + self.client = DicomApiHttpClient() + else: + self.client = client Review comment: it's not that important, but you can do `self.client = client or DicomApiHttpClient()` Feel free to add it if you have the time. ########## File path: sdks/python/setup.py ########## @@ -128,6 +128,7 @@ def get_version(): cythonize = lambda *args, **kwargs: [] REQUIRED_PACKAGES = [ + 'google-auth>=1.18.0,<=1.20.0', Review comment: ah that's right. You need to skip the test when GCP dependencies are missing. Check how we do it for gcs filesystem: https://github.com/apache/beam/blob/master/sdks/python/apache_beam/io/gcp/gcsfilesystem_test.py#L39-L47 ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org