calvinleungyk commented on a change in pull request #15105:
URL: https://github.com/apache/beam/pull/15105#discussion_r671080171



##########
File path: sdks/python/apache_beam/runners/portability/artifact_service.py
##########
@@ -77,8 +79,10 @@ def GetArtifact(self, request, context=None):
     elif request.artifact.type_urn == common_urns.artifact_types.URL.urn:
       payload = proto_utils.parse_Bytes(
           request.artifact.type_payload, 
beam_runner_api_pb2.ArtifactUrlPayload)
-      # TODO(Py3): Remove the unneeded contextlib wrapper.
-      read_handle = contextlib.closing(urlopen(payload.url))
+      if FileSystems.get_scheme(payload.url) == GCSFileSystem.scheme():

Review comment:
       Done. Should we expect the user to provide appropriate URLs? I guess if 
they don't then they will run into an error. Currently supported ones are http, 
https, aws, gcs, azure, hdfs. There's also a LocalFileSystem but I don't think 
folks would be supplying local file paths. If they do that they will just run 
into the error.




-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to