GitHub PullRequestReviewCommentEvent at 2014-08-14 10:56:20
Repository: ivory
Actor: markhibberd
Payload: {"action":"created","comment":{"url":"https://api.github.com/repos/ambiata/ivory/pulls/comments/16231863","id":16231863,"diff_hunk":"@@ -0,0 +1,112 @@\n+package com.ambiata.ivory.operation.rename\n+\n+import java.lang.{Iterable => JIterable}\n+\n+import com.ambiata.ivory.core.thrift.{ThriftFact, ThriftSerialiser}\n+import com.ambiata.ivory.lookup._\n+import com.ambiata.ivory.mr._\n+import com.ambiata.ivory.operation.extraction.SnapshotMapper.ValueState\n+import com.ambiata.ivory.operation.extraction._\n+import com.ambiata.ivory.storage.lookup.ReducerLookups\n+import com.ambiata.ivory.storage.task.FactsetJobKeys\n+import org.apache.hadoop.io._\n+import org.apache.hadoop.mapreduce.lib.output.MultipleOutputs\n+import org.apache.hadoop.mapreduce.{Mapper, Reducer}\n+\n+class RenameMapper extends Mapper[NullWritable, BytesWritable, LongIntWritable, BytesWritable] {\n+\n+  type MapperContext = Mapper[NullWritable, BytesWritable, LongIntWritable, BytesWritable]#Context\n+\n+  /* Value state management, create once per mapper */\n+  var vstate: ValueState = null\n+\n+  /* The output key, only create once per mapper. */\n+  val kout = new LongIntWritable()\n+\n+  /* The output value, only create once per mapper. */\n+  val vout = Writables.bytesWritable(4096)\n+\n+  /* FactsetId created from input split path, only created once per mapper */\n+  val tfact = new ThriftFact\n+\n+  val serializer = new ThriftSerialiser\n+\n+  val mapping: FeatureIdMapping = new FeatureIdMapping()\n+\n+  var converter: VersionedFactConverter = null\n+\n+  override def setup(context: MapperContext): Unit = {\n+    val ctx = MrContext.fromConfiguration(context.getConfiguration)\n+    val (_, vfc, vs) = SnapshotFactsetMapper.setupVersionAndPriority(ctx.thriftCache, context.getConfiguration,\n+      context.getInputSplit)\n+    converter = vfc\n+    vstate = vs\n+    ctx.thriftCache.pop(context.getConfiguration, RenameJob.Keys.Mapping, mapping)\n+  }\n+\n+\n+  override def map(key: NullWritable, value: BytesWritable, context: MapperContext): Unit = {\n+    serializer.fromBytesViewUnsafe(tfact, value.getBytes, 0, value.getLength)\n+    val f = converter.convert(tfact)\n+    val to = mapping.getMapping.get(f.featureId.toString)\n+    if (to != null) {\n+      /***************************************************************\n+       * This is the actual business logic, just in case you missed it\n+       ***************************************************************/\n+      f.toThrift.setAttribute(to.newName)\n+\n+      kout.set((to.ns.toLong << 32) | f.date.int.toLong, f.time.seconds)\n+\n+      vstate.set(f.toThrift, vout)\n+\n+      context.write(kout, vout)\n+      context.getCounter(\"ivory\", \"rename-map\").increment(1)\n+    }\n+  }\n+}\n+\n+class RenameReducer extends Reducer[LongIntWritable, BytesWritable, NullWritable, BytesWritable] {\n+\n+  type ReducerContext = Reducer[LongIntWritable, BytesWritable, NullWritable, BytesWritable]#Context\n+\n+  /* Empty Fact, created once per reducer and mutated per record */\n+  val priority = new PriorityTagDeserializer(new ThriftFact, RenameReducer.ReduceState)","path":"ivory-operation/src/main/scala/com/ambiata/ivory/operation/rename/RenameMapReduce.scala","position":73,"original_position":73,"commit_id":"a15099bf944996581d54f9f9890c2b60f030725a","original_commit_id":"a15099bf944996581d54f9f9890c2b60f030725a","user":{"login":"markhibberd","id":22832,"avatar_url":"https://avatars.githubusercontent.com/u/22832?v=2","gravatar_id":"42d9867a0fee0fa6de6534e9df0f1e9b","url":"https://api.github.com/users/markhibberd","html_url":"https://github.com/markhibberd","followers_url":"https://api.github.com/users/markhibberd/followers","following_url":"https://api.github.com/users/markhibberd/following{/other_user}","gists_url":"https://api.github.com/users/markhibberd/gists{/gist_id}","starred_url":"https://api.github.com/users/markhibberd/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/markhibberd/subscriptions","organizations_url":"https://api.github.com/users/markhibberd/orgs","repos_url":"https://api.github.com/users/markhibberd/repos","events_url":"https://api.github.com/users/markhibberd/events{/privacy}","received_events_url":"https://api.github.com/users/markhibberd/received_events","type":"User","site_admin":false},"body":"Also, we shouldn't have lazy thunks containing mutable state, it is too hard to reason about, even in isolation, can we make it a function () => if it needs to be lazy (but I don't think it does, there really should only ever be 1. not 1 per key).","created_at":"2014-08-14T10:56:20Z","updated_at":"2014-08-14T10:56:20Z","html_url":"https://github.com/ambiata/ivory/pull/204#discussion_r16231863","pull_request_url":"https://api.github.com/repos/ambiata/ivory/pulls/204","_links":{"self":{"href":"https://api.github.com/repos/ambiata/ivory/pulls/comments/16231863"},"html":{"href":"https://github.com/ambiata/ivory/pull/204#discussion_r16231863"},"pull_request":{"href":"https://api.github.com/repos/ambiata/ivory/pulls/204"}}},"pull_request":{"url":"https://api.github.com/repos/ambiata/ivory/pulls/204","id":19658046,"html_url":"https://github.com/ambiata/ivory/pull/204","diff_url":"https://github.com/ambiata/ivory/pull/204.diff","patch_url":"https://github.com/ambiata/ivory/pull/204.patch","issue_url":"https://api.github.com/repos/ambiata/ivory/issues/204","number":204,"state":"open","locked":false,"title":"Added ability to rename features","user":{"login":"charleso","id":31059,"avatar_url":"https://avatars.githubusercontent.com/u/31059?v=2","gravatar_id":"e66f98c1452292a4f7bfe5ee4795624b","url":"https://api.github.com/users/charleso","html_url":"https://github.com/charleso","followers_url":"https://api.github.com/users/charleso/followers","following_url":"https://api.github.com/users/charleso/following{/other_user}","gists_url":"https://api.github.com/users/charleso/gists{/gist_id}","starred_url":"https://api.github.com/users/charleso/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/charleso/subscriptions","organizations_url":"https://api.github.com/users/charleso/orgs","repos_url":"https://api.github.com/users/charleso/repos","events_url":"https://api.github.com/users/charleso/events{/privacy}","received_events_url":"https://api.github.com/users/charleso/received_events","type":"User","site_admin":false},"body":"I'm not completely satisfied with this, but wanted to get it open sooner rather than later. I'll leave a few inline comments. The branch obviously won't build until we do something with `IvoryT` (I really wish you could re-target PRs in GH)\r\n\r\n/cc @etorreborre @markhibberd @raronson ","created_at":"2014-08-12T12:21:51Z","updated_at":"2014-08-14T10:56:20Z","closed_at":null,"merged_at":null,"merge_commit_sha":"a380fcc2f54cafb8f27c85ddae161d0a771e7819","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/ambiata/ivory/pulls/204/commits","review_comments_url":"https://api.github.com/repos/ambiata/ivory/pulls/204/comments","review_comment_url":"https://api.github.com/repos/ambiata/ivory/pulls/comments/{number}","comments_url":"https://api.github.com/repos/ambiata/ivory/issues/204/comments","statuses_url":"https://api.github.com/repos/ambiata/ivory/statuses/a15099bf944996581d54f9f9890c2b60f030725a","head":{"label":"ambiata:topic/factset-rename","ref":"topic/factset-rename","sha":"a15099bf944996581d54f9f9890c2b60f030725a","user":{"login":"ambiata","id":1691023,"avatar_url":"https://avatars.githubusercontent.com/u/1691023?v=2","gravatar_id":"61853572a1cc7fac104c8bb025692149","url":"https://api.github.com/users/ambiata","html_url":"https://github.com/ambiata","followers_url":"https://api.github.com/users/ambiata/followers","following_url":"https://api.github.com/users/ambiata/following{/other_user}","gists_url":"https://api.github.com/users/ambiata/gists{/gist_id}","starred_url":"https://api.github.com/users/ambiata/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ambiata/subscriptions","organizations_url":"https://api.github.com/users/ambiata/orgs","repos_url":"https://api.github.com/users/ambiata/repos","events_url":"https://api.github.com/users/ambiata/events{/privacy}","received_events_url":"https://api.github.com/users/ambiata/received_events","type":"Organization","site_admin":false},"repo":{"id":18756054,"name":"ivory","full_name":"ambiata/ivory","owner":{"login":"ambiata","id":1691023,"avatar_url":"https://avatars.githubusercontent.com/u/1691023?v=2","gravatar_id":"61853572a1cc7fac104c8bb025692149","url":"https://api.github.com/users/ambiata","html_url":"https://github.com/ambiata","followers_url":"https://api.github.com/users/ambiata/followers","following_url":"https://api.github.com/users/ambiata/following{/other_user}","gists_url":"https://api.github.com/users/ambiata/gists{/gist_id}","starred_url":"https://api.github.com/users/ambiata/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ambiata/subscriptions","organizations_url":"https://api.github.com/users/ambiata/orgs","repos_url":"https://api.github.com/users/ambiata/repos","events_url":"https://api.github.com/users/ambiata/events{/privacy}","received_events_url":"https://api.github.com/users/ambiata/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/ambiata/ivory","description":"A scalable feature store.","fork":false,"url":"https://api.github.com/repos/ambiata/ivory","forks_url":"https://api.github.com/repos/ambiata/ivory/forks","keys_url":"https://api.github.com/repos/ambiata/ivory/keys{/key_id}","collaborators_url":"https://api.github.com/repos/ambiata/ivory/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/ambiata/ivory/teams","hooks_url":"https://api.github.com/repos/ambiata/ivory/hooks","issue_events_url":"https://api.github.com/repos/ambiata/ivory/issues/events{/number}","events_url":"https://api.github.com/repos/ambiata/ivory/events","assignees_url":"https://api.github.com/repos/ambiata/ivory/assignees{/user}","branches_url":"https://api.github.com/repos/ambiata/ivory/branches{/branch}","tags_url":"https://api.github.com/repos/ambiata/ivory/tags","blobs_url":"https://api.github.com/repos/ambiata/ivory/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/ambiata/ivory/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/ambiata/ivory/git/refs{/sha}","trees_url":"https://api.github.com/repos/ambiata/ivory/git/trees{/sha}","statuses_url":"https://api.github.com/repos/ambiata/ivory/statuses/{sha}","languages_url":"https://api.github.com/repos/ambiata/ivory/languages","stargazers_url":"https://api.github.com/repos/ambiata/ivory/stargazers","contributors_url":"https://api.github.com/repos/ambiata/ivory/contributors","subscribers_url":"https://api.github.com/repos/ambiata/ivory/subscribers","subscription_url":"https://api.github.com/repos/ambiata/ivory/subscription","commits_url":"https://api.github.com/repos/ambiata/ivory/commits{/sha}","git_commits_url":"https://api.github.com/repos/ambiata/ivory/git/commits{/sha}","comments_url":"https://api.github.com/repos/ambiata/ivory/comments{/number}","issue_comment_url":"https://api.github.com/repos/ambiata/ivory/issues/comments/{number}","contents_url":"https://api.github.com/repos/ambiata/ivory/contents/{+path}","compare_url":"https://api.github.com/repos/ambiata/ivory/compare/{base}...{head}","merges_url":"https://api.github.com/repos/ambiata/ivory/merges","archive_url":"https://api.github.com/repos/ambiata/ivory/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/ambiata/ivory/downloads","issues_url":"https://api.github.com/repos/ambiata/ivory/issues{/number}","pulls_url":"https://api.github.com/repos/ambiata/ivory/pulls{/number}","milestones_url":"https://api.github.com/repos/ambiata/ivory/milestones{/number}","notifications_url":"https://api.github.com/repos/ambiata/ivory/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/ambiata/ivory/labels{/name}","releases_url":"https://api.github.com/repos/ambiata/ivory/releases{/id}","created_at":"2014-04-14T09:54:53Z","updated_at":"2014-08-05T00:29:07Z","pushed_at":"2014-08-14T08:41:58Z","git_url":"git://github.com/ambiata/ivory.git","ssh_url":"git@github.com:ambiata/ivory.git","clone_url":"https://github.com/ambiata/ivory.git","svn_url":"https://github.com/ambiata/ivory","homepage":null,"size":9419,"stargazers_count":13,"watchers_count":13,"language":"Java","has_issues":true,"has_downloads":true,"has_wiki":true,"forks_count":3,"mirror_url":null,"open_issues_count":64,"forks":3,"open_issues":64,"watchers":13,"default_branch":"master"}},"base":{"label":"ambiata:master","ref":"master","sha":"7635044c041c1298b7f7a816cfce8ca621e1778f","user":{"login":"ambiata","id":1691023,"avatar_url":"https://avatars.githubusercontent.com/u/1691023?v=2","gravatar_id":"61853572a1cc7fac104c8bb025692149","url":"https://api.github.com/users/ambiata","html_url":"https://github.com/ambiata","followers_url":"https://api.github.com/users/ambiata/followers","following_url":"https://api.github.com/users/ambiata/following{/other_user}","gists_url":"https://api.github.com/users/ambiata/gists{/gist_id}","starred_url":"https://api.github.com/users/ambiata/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ambiata/subscriptions","organizations_url":"https://api.github.com/users/ambiata/orgs","repos_url":"https://api.github.com/users/ambiata/repos","events_url":"https://api.github.com/users/ambiata/events{/privacy}","received_events_url":"https://api.github.com/users/ambiata/received_events","type":"Organization","site_admin":false},"repo":{"id":18756054,"name":"ivory","full_name":"ambiata/ivory","owner":{"login":"ambiata","id":1691023,"avatar_url":"https://avatars.githubusercontent.com/u/1691023?v=2","gravatar_id":"61853572a1cc7fac104c8bb025692149","url":"https://api.github.com/users/ambiata","html_url":"https://github.com/ambiata","followers_url":"https://api.github.com/users/ambiata/followers","following_url":"https://api.github.com/users/ambiata/following{/other_user}","gists_url":"https://api.github.com/users/ambiata/gists{/gist_id}","starred_url":"https://api.github.com/users/ambiata/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ambiata/subscriptions","organizations_url":"https://api.github.com/users/ambiata/orgs","repos_url":"https://api.github.com/users/ambiata/repos","events_url":"https://api.github.com/users/ambiata/events{/privacy}","received_events_url":"https://api.github.com/users/ambiata/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/ambiata/ivory","description":"A scalable feature store.","fork":false,"url":"https://api.github.com/repos/ambiata/ivory","forks_url":"https://api.github.com/repos/ambiata/ivory/forks","keys_url":"https://api.github.com/repos/ambiata/ivory/keys{/key_id}","collaborators_url":"https://api.github.com/repos/ambiata/ivory/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/ambiata/ivory/teams","hooks_url":"https://api.github.com/repos/ambiata/ivory/hooks","issue_events_url":"https://api.github.com/repos/ambiata/ivory/issues/events{/number}","events_url":"https://api.github.com/repos/ambiata/ivory/events","assignees_url":"https://api.github.com/repos/ambiata/ivory/assignees{/user}","branches_url":"https://api.github.com/repos/ambiata/ivory/branches{/branch}","tags_url":"https://api.github.com/repos/ambiata/ivory/tags","blobs_url":"https://api.github.com/repos/ambiata/ivory/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/ambiata/ivory/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/ambiata/ivory/git/refs{/sha}","trees_url":"https://api.github.com/repos/ambiata/ivory/git/trees{/sha}","statuses_url":"https://api.github.com/repos/ambiata/ivory/statuses/{sha}","languages_url":"https://api.github.com/repos/ambiata/ivory/languages","stargazers_url":"https://api.github.com/repos/ambiata/ivory/stargazers","contributors_url":"https://api.github.com/repos/ambiata/ivory/contributors","subscribers_url":"https://api.github.com/repos/ambiata/ivory/subscribers","subscription_url":"https://api.github.com/repos/ambiata/ivory/subscription","commits_url":"https://api.github.com/repos/ambiata/ivory/commits{/sha}","git_commits_url":"https://api.github.com/repos/ambiata/ivory/git/commits{/sha}","comments_url":"https://api.github.com/repos/ambiata/ivory/comments{/number}","issue_comment_url":"https://api.github.com/repos/ambiata/ivory/issues/comments/{number}","contents_url":"https://api.github.com/repos/ambiata/ivory/contents/{+path}","compare_url":"https://api.github.com/repos/ambiata/ivory/compare/{base}...{head}","merges_url":"https://api.github.com/repos/ambiata/ivory/merges","archive_url":"https://api.github.com/repos/ambiata/ivory/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/ambiata/ivory/downloads","issues_url":"https://api.github.com/repos/ambiata/ivory/issues{/number}","pulls_url":"https://api.github.com/repos/ambiata/ivory/pulls{/number}","milestones_url":"https://api.github.com/repos/ambiata/ivory/milestones{/number}","notifications_url":"https://api.github.com/repos/ambiata/ivory/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/ambiata/ivory/labels{/name}","releases_url":"https://api.github.com/repos/ambiata/ivory/releases{/id}","created_at":"2014-04-14T09:54:53Z","updated_at":"2014-08-05T00:29:07Z","pushed_at":"2014-08-14T08:41:58Z","git_url":"git://github.com/ambiata/ivory.git","ssh_url":"git@github.com:ambiata/ivory.git","clone_url":"https://github.com/ambiata/ivory.git","svn_url":"https://github.com/ambiata/ivory","homepage":null,"size":9419,"stargazers_count":13,"watchers_count":13,"language":"Java","has_issues":true,"has_downloads":true,"has_wiki":true,"forks_count":3,"mirror_url":null,"open_issues_count":64,"forks":3,"open_issues":64,"watchers":13,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/ambiata/ivory/pulls/204"},"html":{"href":"https://github.com/ambiata/ivory/pull/204"},"issue":{"href":"https://api.github.com/repos/ambiata/ivory/issues/204"},"comments":{"href":"https://api.github.com/repos/ambiata/ivory/issues/204/comments"},"review_comments":{"href":"https://api.github.com/repos/ambiata/ivory/pulls/204/comments"},"review_comment":{"href":"https://api.github.com/repos/ambiata/ivory/pulls/comments/{number}"},"commits":{"href":"https://api.github.com/repos/ambiata/ivory/pulls/204/commits"},"statuses":{"href":"https://api.github.com/repos/ambiata/ivory/statuses/a15099bf944996581d54f9f9890c2b60f030725a"}}}}