{"action":"created","comment":{"url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/pulls/comments/87518436","pull_request_review_id":8129385,"id":87518436,"diff_hunk":"@@ -370,21 +390,18 @@ public void finalize(Iterable<FileResult> writerResults, PipelineOptions options\n     }\n \n     /**\n-     * Removes temporary output files. Uses the temporary filename to find files to remove.\n+     * Removes temporary output files. Uses the temporary directory to find files to remove.\n      *\n      * <p>Can be called from subclasses that override {@link FileBasedWriteOperation#finalize}.\n      * <b>Note:</b>If finalize is overridden and does <b>not</b> rename or otherwise finalize\n      * temporary files, this method will remove them.\n      */\n-    protected final void removeTemporaryFiles(PipelineOptions options) throws IOException {\n-      String pattern = buildTemporaryFilename(baseTemporaryFilename, \"*\");\n-      LOG.debug(\"Finding temporary bundle output files matching {}.\", pattern);\n-      FileOperations fileOperations = FileOperationsFactory.getFileOperations(pattern, options);\n-      IOChannelFactory factory = IOChannelUtils.getFactory(pattern);\n-      Collection<String> matches = factory.match(pattern);\n-      LOG.debug(\"{} temporary files matched {}\", matches.size(), pattern);\n-      LOG.debug(\"Removing {} files.\", matches.size());\n-      fileOperations.remove(matches);\n+    protected final void removeTemporaryFiles(List<String> knownFiles, PipelineOptions options)","path":"sdk/src/main/java/com/google/cloud/dataflow/sdk/io/FileBasedSink.java","position":186,"original_position":186,"commit_id":"e469a22a5db41766589257b0c49cd0cca1ecc4d7","original_commit_id":"e469a22a5db41766589257b0c49cd0cca1ecc4d7","user":{"login":"jkff","id":343722,"avatar_url":"https://avatars.githubusercontent.com/u/343722?v=3","gravatar_id":"","url":"https://api.github.com/users/jkff","html_url":"https://github.com/jkff","followers_url":"https://api.github.com/users/jkff/followers","following_url":"https://api.github.com/users/jkff/following{/other_user}","gists_url":"https://api.github.com/users/jkff/gists{/gist_id}","starred_url":"https://api.github.com/users/jkff/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jkff/subscriptions","organizations_url":"https://api.github.com/users/jkff/orgs","repos_url":"https://api.github.com/users/jkff/repos","events_url":"https://api.github.com/users/jkff/events{/privacy}","received_events_url":"https://api.github.com/users/jkff/received_events","type":"User","site_admin":false},"body":"The rest of the current change is technically also not backward-compatible (e.g. the set of protected member variables has changed) for someone who was overriding FileBasedSink/FileBasedWriteOperation/... in a highly non-trivial way (e.g. trying to construct filenames manually using the protected members).\r\n\r\nNote that, of the file-based sinks I've seen, none would be broken by this change, because they don't peek into these protected variables.\r\n\r\nHowever, as for those that potentially do, I don't think there's a way to make the current change backward-compatible for them. There's a choice:\r\n\r\n- Make the current change, with a (I think) very small chance of breaking somebody, but in return, fix deletion of files and data loss/duplication issues.\r\n- Abandon the current change and say that the problem is only fixed in the Beam SDK\r\n- Do something much weaker but backward-compatible instead: e.g., simply add code for removing all known files, but do not place temp files in a subdirectory. As an unfortunate side effect, this will cause a divergence in code and behavior between Dataflow and Beam SDKs.\r\n\r\nWhich would you prefer? Are there other options?","created_at":"2016-11-11T00:45:42Z","updated_at":"2016-11-11T00:45:42Z","html_url":"https://github.com/GoogleCloudPlatform/DataflowJavaSDK/pull/482#discussion_r87518436","pull_request_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/pulls/482","_links":{"self":{"href":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/pulls/comments/87518436"},"html":{"href":"https://github.com/GoogleCloudPlatform/DataflowJavaSDK/pull/482#discussion_r87518436"},"pull_request":{"href":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/pulls/482"}}},"pull_request":{"url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/pulls/482","id":93236851,"html_url":"https://github.com/GoogleCloudPlatform/DataflowJavaSDK/pull/482","diff_url":"https://github.com/GoogleCloudPlatform/DataflowJavaSDK/pull/482.diff","patch_url":"https://github.com/GoogleCloudPlatform/DataflowJavaSDK/pull/482.patch","issue_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/issues/482","number":482,"state":"open","locked":false,"title":"Backporting FileBasedSink changes from Beam","user":{"login":"jkff","id":343722,"avatar_url":"https://avatars.githubusercontent.com/u/343722?v=3","gravatar_id":"","url":"https://api.github.com/users/jkff","html_url":"https://github.com/jkff","followers_url":"https://api.github.com/users/jkff/followers","following_url":"https://api.github.com/users/jkff/following{/other_user}","gists_url":"https://api.github.com/users/jkff/gists{/gist_id}","starred_url":"https://api.github.com/users/jkff/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jkff/subscriptions","organizations_url":"https://api.github.com/users/jkff/orgs","repos_url":"https://api.github.com/users/jkff/repos","events_url":"https://api.github.com/users/jkff/events{/privacy}","received_events_url":"https://api.github.com/users/jkff/received_events","type":"User","site_admin":false},"body":"Original PRs:\r\nhttps://github.com/apache/incubator-beam/pull/1050\r\nhttps://github.com/apache/incubator-beam/pull/1278\r\n\r\nChanges simply copied over. No modifications.\r\n\r\nR: @lukecwik ","created_at":"2016-11-10T21:45:15Z","updated_at":"2016-11-11T00:45:42Z","closed_at":null,"merged_at":null,"merge_commit_sha":"c71b1cd90370061e439e04b2394f7209e02f59fa","assignee":null,"assignees":[],"milestone":null,"commits_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/pulls/482/commits","review_comments_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/pulls/482/comments","review_comment_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/pulls/comments{/number}","comments_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/issues/482/comments","statuses_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/statuses/e469a22a5db41766589257b0c49cd0cca1ecc4d7","head":{"label":"jkff:backport-1278","ref":"backport-1278","sha":"e469a22a5db41766589257b0c49cd0cca1ecc4d7","user":{"login":"jkff","id":343722,"avatar_url":"https://avatars.githubusercontent.com/u/343722?v=3","gravatar_id":"","url":"https://api.github.com/users/jkff","html_url":"https://github.com/jkff","followers_url":"https://api.github.com/users/jkff/followers","following_url":"https://api.github.com/users/jkff/following{/other_user}","gists_url":"https://api.github.com/users/jkff/gists{/gist_id}","starred_url":"https://api.github.com/users/jkff/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jkff/subscriptions","organizations_url":"https://api.github.com/users/jkff/orgs","repos_url":"https://api.github.com/users/jkff/repos","events_url":"https://api.github.com/users/jkff/events{/privacy}","received_events_url":"https://api.github.com/users/jkff/received_events","type":"User","site_admin":false},"repo":{"id":53903032,"name":"DataflowJavaSDK","full_name":"jkff/DataflowJavaSDK","owner":{"login":"jkff","id":343722,"avatar_url":"https://avatars.githubusercontent.com/u/343722?v=3","gravatar_id":"","url":"https://api.github.com/users/jkff","html_url":"https://github.com/jkff","followers_url":"https://api.github.com/users/jkff/followers","following_url":"https://api.github.com/users/jkff/following{/other_user}","gists_url":"https://api.github.com/users/jkff/gists{/gist_id}","starred_url":"https://api.github.com/users/jkff/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jkff/subscriptions","organizations_url":"https://api.github.com/users/jkff/orgs","repos_url":"https://api.github.com/users/jkff/repos","events_url":"https://api.github.com/users/jkff/events{/privacy}","received_events_url":"https://api.github.com/users/jkff/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/jkff/DataflowJavaSDK","description":"Google Cloud Dataflow provides a simple, powerful model for building both batch and streaming parallel data processing pipelines.","fork":true,"url":"https://api.github.com/repos/jkff/DataflowJavaSDK","forks_url":"https://api.github.com/repos/jkff/DataflowJavaSDK/forks","keys_url":"https://api.github.com/repos/jkff/DataflowJavaSDK/keys{/key_id}","collaborators_url":"https://api.github.com/repos/jkff/DataflowJavaSDK/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/jkff/DataflowJavaSDK/teams","hooks_url":"https://api.github.com/repos/jkff/DataflowJavaSDK/hooks","issue_events_url":"https://api.github.com/repos/jkff/DataflowJavaSDK/issues/events{/number}","events_url":"https://api.github.com/repos/jkff/DataflowJavaSDK/events","assignees_url":"https://api.github.com/repos/jkff/DataflowJavaSDK/assignees{/user}","branches_url":"https://api.github.com/repos/jkff/DataflowJavaSDK/branches{/branch}","tags_url":"https://api.github.com/repos/jkff/DataflowJavaSDK/tags","blobs_url":"https://api.github.com/repos/jkff/DataflowJavaSDK/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/jkff/DataflowJavaSDK/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/jkff/DataflowJavaSDK/git/refs{/sha}","trees_url":"https://api.github.com/repos/jkff/DataflowJavaSDK/git/trees{/sha}","statuses_url":"https://api.github.com/repos/jkff/DataflowJavaSDK/statuses/{sha}","languages_url":"https://api.github.com/repos/jkff/DataflowJavaSDK/languages","stargazers_url":"https://api.github.com/repos/jkff/DataflowJavaSDK/stargazers","contributors_url":"https://api.github.com/repos/jkff/DataflowJavaSDK/contributors","subscribers_url":"https://api.github.com/repos/jkff/DataflowJavaSDK/subscribers","subscription_url":"https://api.github.com/repos/jkff/DataflowJavaSDK/subscription","commits_url":"https://api.github.com/repos/jkff/DataflowJavaSDK/commits{/sha}","git_commits_url":"https://api.github.com/repos/jkff/DataflowJavaSDK/git/commits{/sha}","comments_url":"https://api.github.com/repos/jkff/DataflowJavaSDK/comments{/number}","issue_comment_url":"https://api.github.com/repos/jkff/DataflowJavaSDK/issues/comments{/number}","contents_url":"https://api.github.com/repos/jkff/DataflowJavaSDK/contents/{+path}","compare_url":"https://api.github.com/repos/jkff/DataflowJavaSDK/compare/{base}...{head}","merges_url":"https://api.github.com/repos/jkff/DataflowJavaSDK/merges","archive_url":"https://api.github.com/repos/jkff/DataflowJavaSDK/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/jkff/DataflowJavaSDK/downloads","issues_url":"https://api.github.com/repos/jkff/DataflowJavaSDK/issues{/number}","pulls_url":"https://api.github.com/repos/jkff/DataflowJavaSDK/pulls{/number}","milestones_url":"https://api.github.com/repos/jkff/DataflowJavaSDK/milestones{/number}","notifications_url":"https://api.github.com/repos/jkff/DataflowJavaSDK/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/jkff/DataflowJavaSDK/labels{/name}","releases_url":"https://api.github.com/repos/jkff/DataflowJavaSDK/releases{/id}","deployments_url":"https://api.github.com/repos/jkff/DataflowJavaSDK/deployments","created_at":"2016-03-15T00:55:07Z","updated_at":"2016-03-15T00:55:10Z","pushed_at":"2016-11-10T21:43:40Z","git_url":"git://github.com/jkff/DataflowJavaSDK.git","ssh_url":"git@github.com:jkff/DataflowJavaSDK.git","clone_url":"https://github.com/jkff/DataflowJavaSDK.git","svn_url":"https://github.com/jkff/DataflowJavaSDK","homepage":"http://cloud.google.com/dataflow","size":12795,"stargazers_count":0,"watchers_count":0,"language":"Java","has_issues":false,"has_downloads":false,"has_wiki":false,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"GoogleCloudPlatform:master","ref":"master","sha":"6706611b8f6794148ba068b4dc5a691f7be9bcff","user":{"login":"GoogleCloudPlatform","id":2810941,"avatar_url":"https://avatars.githubusercontent.com/u/2810941?v=3","gravatar_id":"","url":"https://api.github.com/users/GoogleCloudPlatform","html_url":"https://github.com/GoogleCloudPlatform","followers_url":"https://api.github.com/users/GoogleCloudPlatform/followers","following_url":"https://api.github.com/users/GoogleCloudPlatform/following{/other_user}","gists_url":"https://api.github.com/users/GoogleCloudPlatform/gists{/gist_id}","starred_url":"https://api.github.com/users/GoogleCloudPlatform/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/GoogleCloudPlatform/subscriptions","organizations_url":"https://api.github.com/users/GoogleCloudPlatform/orgs","repos_url":"https://api.github.com/users/GoogleCloudPlatform/repos","events_url":"https://api.github.com/users/GoogleCloudPlatform/events{/privacy}","received_events_url":"https://api.github.com/users/GoogleCloudPlatform/received_events","type":"Organization","site_admin":false},"repo":{"id":27503425,"name":"DataflowJavaSDK","full_name":"GoogleCloudPlatform/DataflowJavaSDK","owner":{"login":"GoogleCloudPlatform","id":2810941,"avatar_url":"https://avatars.githubusercontent.com/u/2810941?v=3","gravatar_id":"","url":"https://api.github.com/users/GoogleCloudPlatform","html_url":"https://github.com/GoogleCloudPlatform","followers_url":"https://api.github.com/users/GoogleCloudPlatform/followers","following_url":"https://api.github.com/users/GoogleCloudPlatform/following{/other_user}","gists_url":"https://api.github.com/users/GoogleCloudPlatform/gists{/gist_id}","starred_url":"https://api.github.com/users/GoogleCloudPlatform/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/GoogleCloudPlatform/subscriptions","organizations_url":"https://api.github.com/users/GoogleCloudPlatform/orgs","repos_url":"https://api.github.com/users/GoogleCloudPlatform/repos","events_url":"https://api.github.com/users/GoogleCloudPlatform/events{/privacy}","received_events_url":"https://api.github.com/users/GoogleCloudPlatform/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/GoogleCloudPlatform/DataflowJavaSDK","description":"Google Cloud Dataflow provides a simple, powerful model for building both batch and streaming parallel data processing pipelines.","fork":false,"url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK","forks_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/forks","keys_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/keys{/key_id}","collaborators_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/teams","hooks_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/hooks","issue_events_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/issues/events{/number}","events_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/events","assignees_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/assignees{/user}","branches_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/branches{/branch}","tags_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/tags","blobs_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/git/refs{/sha}","trees_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/git/trees{/sha}","statuses_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/statuses/{sha}","languages_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/languages","stargazers_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/stargazers","contributors_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/contributors","subscribers_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/subscribers","subscription_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/subscription","commits_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/commits{/sha}","git_commits_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/git/commits{/sha}","comments_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/comments{/number}","issue_comment_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/issues/comments{/number}","contents_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/contents/{+path}","compare_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/compare/{base}...{head}","merges_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/merges","archive_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/downloads","issues_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/issues{/number}","pulls_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/pulls{/number}","milestones_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/milestones{/number}","notifications_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/labels{/name}","releases_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/releases{/id}","deployments_url":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/deployments","created_at":"2014-12-03T19:20:21Z","updated_at":"2016-11-01T06:05:28Z","pushed_at":"2016-11-10T21:45:15Z","git_url":"git://github.com/GoogleCloudPlatform/DataflowJavaSDK.git","ssh_url":"git@github.com:GoogleCloudPlatform/DataflowJavaSDK.git","clone_url":"https://github.com/GoogleCloudPlatform/DataflowJavaSDK.git","svn_url":"https://github.com/GoogleCloudPlatform/DataflowJavaSDK","homepage":"http://cloud.google.com/dataflow","size":14632,"stargazers_count":696,"watchers_count":696,"language":"Java","has_issues":true,"has_downloads":false,"has_wiki":false,"has_pages":false,"forks_count":280,"mirror_url":null,"open_issues_count":31,"forks":280,"open_issues":31,"watchers":696,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/pulls/482"},"html":{"href":"https://github.com/GoogleCloudPlatform/DataflowJavaSDK/pull/482"},"issue":{"href":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/issues/482"},"comments":{"href":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/issues/482/comments"},"review_comments":{"href":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/pulls/482/comments"},"review_comment":{"href":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/pulls/482/commits"},"statuses":{"href":"https://api.github.com/repos/GoogleCloudPlatform/DataflowJavaSDK/statuses/e469a22a5db41766589257b0c49cd0cca1ecc4d7"}}}}
{"actor":{"display_login":"jkff"}}