{"action":"created","comment":{"url":"https://api.github.com/repos/broadinstitute/rawls/pulls/comments/38325459","id":38325459,"diff_hunk":"@@ -26,63 +28,69 @@ object MethodConfigResolver {\n     case _ => Failure(new MultipleResultException)\n   }\n \n+  private def getArrayResult(seq: Seq[AttributeValue]): Try[AttributeValueList] = {\n+    // TODO figure out if this is the right approach to filtering nulls.\n+    Success(AttributeValueList(seq.filter(v => v != null && v != AttributeNull)))\n+  }\n+\n   /**\n    * Evaluate expressions in a method config against a root entity.\n-   * @return map from input name to a Try containing a resolved value or failure\n+   * @return map from method config input name to a Try containing a resolved value or failure\n    */\n-  def resolveInputs(workspaceContext: WorkspaceContext, methodConfig: MethodConfiguration, entity: Entity, wdl: String, txn: RawlsTransaction): Map[String, Try[Attribute]] = {\n-    val wdlInputs = NamespaceWithWorkflow.load(wdl, BackendType.LOCAL).workflow.inputs.map((w:WorkflowInput) => w.fqn -> w.wdlType).toMap\n+  def evaluateMethodConfigInputs(workspaceContext: WorkspaceContext, methodConfig: MethodConfiguration, entity: Entity, txn: RawlsTransaction): Map[String, Try[Seq[AttributeValue]]] = {\n     txn withGraph { graph =>\n       val evaluator = new ExpressionEvaluator(new ExpressionParser)\n       methodConfig.inputs.map {\n         case (name, expression) => {\n           val evaluated = evaluator.evalFinalAttribute(workspaceContext, entity.entityType, entity.name, expression.value)\n-          // now look at expected WDL type to see if we should unpack a single value from the Seq[Any]\n-          val unpacked = evaluated.flatMap(sequence => wdlInputs.get(name) match {\n-              // TODO: Cromwell doesn't yet have compound types (e.g. Seq)\n-              // TODO: so until it does just take the head\n-              case Some(wdlType) => getSingleResult(sequence)\n-              case None => getSingleResult(sequence)\n-            }\n-          )\n-          (name, unpacked)\n+          (name, evaluated)\n         }\n       }\n     }\n   }\n \n   /**\n-   * Get errors in both expression eval and WDL binding.\n-   * @return map of from input name to error message (so if the map is empty, the config is valid)\n+   * Unpack evaluated method config inputs using WDL.\n+   * @return map from workflow input name to a Try containing an unpacked value or failure\n    */\n-  def getValidationErrors(workspaceContext: WorkspaceContext, methodConfig: MethodConfiguration, entity: Entity, wdl: String, txn: RawlsTransaction) = {\n-    val configInputs = resolveInputs(workspaceContext, methodConfig, entity, wdl, txn)\n-    NamespaceWithWorkflow.load(wdl, BackendType.LOCAL).workflow.inputs.map {\n-      case w:WorkflowInput => {\n-        val errorOption = configInputs.get(w.fqn) match {\n-          case Some(Success(rawValue)) => {\n-            // TODO: currently Cromwell's exposed type-checking is not very robust.\n-            // TODO: once Cromwell has a validation endpoint, use that; until then, don't bother checking types.\n-            None\n+  def unpackWorkflowInputs(mcInputs: Map[String, Try[Seq[AttributeValue]]], wdl: String): Map[String, Try[Attribute]] = {","path":"src/main/scala/org/broadinstitute/dsde/rawls/jobexec/MethodConfigResolver.scala","position":76,"original_position":76,"commit_id":"a7ce0f73ca16535953d02191ae12923472728077","original_commit_id":"a7ce0f73ca16535953d02191ae12923472728077","user":{"login":"dvoet","id":1521333,"avatar_url":"https://avatars.githubusercontent.com/u/1521333?v=3","gravatar_id":"","url":"https://api.github.com/users/dvoet","html_url":"https://github.com/dvoet","followers_url":"https://api.github.com/users/dvoet/followers","following_url":"https://api.github.com/users/dvoet/following{/other_user}","gists_url":"https://api.github.com/users/dvoet/gists{/gist_id}","starred_url":"https://api.github.com/users/dvoet/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dvoet/subscriptions","organizations_url":"https://api.github.com/users/dvoet/orgs","repos_url":"https://api.github.com/users/dvoet/repos","events_url":"https://api.github.com/users/dvoet/events{/privacy}","received_events_url":"https://api.github.com/users/dvoet/received_events","type":"User","site_admin":false},"body":"This signature seems funny to me. First mcInputs has Seq[AttributeValue] but the return has just Attribute. Second, by the time this is called, I would expect every element in mcInputs to be Success so why do we have the Try at all?","created_at":"2015-08-31T15:46:15Z","updated_at":"2015-08-31T15:46:15Z","html_url":"https://github.com/broadinstitute/rawls/pull/101#discussion_r38325459","pull_request_url":"https://api.github.com/repos/broadinstitute/rawls/pulls/101","_links":{"self":{"href":"https://api.github.com/repos/broadinstitute/rawls/pulls/comments/38325459"},"html":{"href":"https://github.com/broadinstitute/rawls/pull/101#discussion_r38325459"},"pull_request":{"href":"https://api.github.com/repos/broadinstitute/rawls/pulls/101"}}},"pull_request":{"url":"https://api.github.com/repos/broadinstitute/rawls/pulls/101","id":43668544,"html_url":"https://github.com/broadinstitute/rawls/pull/101","diff_url":"https://github.com/broadinstitute/rawls/pull/101.diff","patch_url":"https://github.com/broadinstitute/rawls/pull/101.patch","issue_url":"https://api.github.com/repos/broadinstitute/rawls/issues/101","number":101,"state":"open","locked":false,"title":"method config validation","user":{"login":"mattsooknah","id":2549178,"avatar_url":"https://avatars.githubusercontent.com/u/2549178?v=3","gravatar_id":"","url":"https://api.github.com/users/mattsooknah","html_url":"https://github.com/mattsooknah","followers_url":"https://api.github.com/users/mattsooknah/followers","following_url":"https://api.github.com/users/mattsooknah/following{/other_user}","gists_url":"https://api.github.com/users/mattsooknah/gists{/gist_id}","starred_url":"https://api.github.com/users/mattsooknah/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mattsooknah/subscriptions","organizations_url":"https://api.github.com/users/mattsooknah/orgs","repos_url":"https://api.github.com/users/mattsooknah/repos","events_url":"https://api.github.com/users/mattsooknah/events{/privacy}","received_events_url":"https://api.github.com/users/mattsooknah/received_events","type":"User","site_admin":false},"body":"Stuff for https://broadinstitute.atlassian.net/browse/DSDEEPB-661.\r\n\r\n* expression evaluation is separate from \"unpacking\" into WDL, which covers things like array vs single types and optional values\r\n* missing / bad expressions are OK as long as the corresponding WDL input is optional (currently I'm representing this by replacing the eval errors with AttributeNull to represent a null value, which is then ignored when sending to Cromwell. Maybe there's a better way to do it)\r\n* fixed the three-step WDL being used by some unit tests, which had deprecated syntax.\r\n* actual full type-checking is not part of the unpacking. That's done by Cromwell when actually submitting a workflow. And the (still unused) validation function now calls Cromwell's validation endpoint.","created_at":"2015-08-28T20:27:00Z","updated_at":"2015-08-31T15:46:15Z","closed_at":null,"merged_at":null,"merge_commit_sha":"0d1bfab4a091719bf2f400e611b943a2f3d1b7b8","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/broadinstitute/rawls/pulls/101/commits","review_comments_url":"https://api.github.com/repos/broadinstitute/rawls/pulls/101/comments","review_comment_url":"https://api.github.com/repos/broadinstitute/rawls/pulls/comments{/number}","comments_url":"https://api.github.com/repos/broadinstitute/rawls/issues/101/comments","statuses_url":"https://api.github.com/repos/broadinstitute/rawls/statuses/a7ce0f73ca16535953d02191ae12923472728077","head":{"label":"broadinstitute:mds_cromwell_validation","ref":"mds_cromwell_validation","sha":"a7ce0f73ca16535953d02191ae12923472728077","user":{"login":"broadinstitute","id":393552,"avatar_url":"https://avatars.githubusercontent.com/u/393552?v=3","gravatar_id":"","url":"https://api.github.com/users/broadinstitute","html_url":"https://github.com/broadinstitute","followers_url":"https://api.github.com/users/broadinstitute/followers","following_url":"https://api.github.com/users/broadinstitute/following{/other_user}","gists_url":"https://api.github.com/users/broadinstitute/gists{/gist_id}","starred_url":"https://api.github.com/users/broadinstitute/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/broadinstitute/subscriptions","organizations_url":"https://api.github.com/users/broadinstitute/orgs","repos_url":"https://api.github.com/users/broadinstitute/repos","events_url":"https://api.github.com/users/broadinstitute/events{/privacy}","received_events_url":"https://api.github.com/users/broadinstitute/received_events","type":"Organization","site_admin":false},"repo":{"id":34129770,"name":"rawls","full_name":"broadinstitute/rawls","owner":{"login":"broadinstitute","id":393552,"avatar_url":"https://avatars.githubusercontent.com/u/393552?v=3","gravatar_id":"","url":"https://api.github.com/users/broadinstitute","html_url":"https://github.com/broadinstitute","followers_url":"https://api.github.com/users/broadinstitute/followers","following_url":"https://api.github.com/users/broadinstitute/following{/other_user}","gists_url":"https://api.github.com/users/broadinstitute/gists{/gist_id}","starred_url":"https://api.github.com/users/broadinstitute/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/broadinstitute/subscriptions","organizations_url":"https://api.github.com/users/broadinstitute/orgs","repos_url":"https://api.github.com/users/broadinstitute/repos","events_url":"https://api.github.com/users/broadinstitute/events{/privacy}","received_events_url":"https://api.github.com/users/broadinstitute/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/broadinstitute/rawls","description":"Rawls service for DSDE","fork":false,"url":"https://api.github.com/repos/broadinstitute/rawls","forks_url":"https://api.github.com/repos/broadinstitute/rawls/forks","keys_url":"https://api.github.com/repos/broadinstitute/rawls/keys{/key_id}","collaborators_url":"https://api.github.com/repos/broadinstitute/rawls/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/broadinstitute/rawls/teams","hooks_url":"https://api.github.com/repos/broadinstitute/rawls/hooks","issue_events_url":"https://api.github.com/repos/broadinstitute/rawls/issues/events{/number}","events_url":"https://api.github.com/repos/broadinstitute/rawls/events","assignees_url":"https://api.github.com/repos/broadinstitute/rawls/assignees{/user}","branches_url":"https://api.github.com/repos/broadinstitute/rawls/branches{/branch}","tags_url":"https://api.github.com/repos/broadinstitute/rawls/tags","blobs_url":"https://api.github.com/repos/broadinstitute/rawls/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/broadinstitute/rawls/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/broadinstitute/rawls/git/refs{/sha}","trees_url":"https://api.github.com/repos/broadinstitute/rawls/git/trees{/sha}","statuses_url":"https://api.github.com/repos/broadinstitute/rawls/statuses/{sha}","languages_url":"https://api.github.com/repos/broadinstitute/rawls/languages","stargazers_url":"https://api.github.com/repos/broadinstitute/rawls/stargazers","contributors_url":"https://api.github.com/repos/broadinstitute/rawls/contributors","subscribers_url":"https://api.github.com/repos/broadinstitute/rawls/subscribers","subscription_url":"https://api.github.com/repos/broadinstitute/rawls/subscription","commits_url":"https://api.github.com/repos/broadinstitute/rawls/commits{/sha}","git_commits_url":"https://api.github.com/repos/broadinstitute/rawls/git/commits{/sha}","comments_url":"https://api.github.com/repos/broadinstitute/rawls/comments{/number}","issue_comment_url":"https://api.github.com/repos/broadinstitute/rawls/issues/comments{/number}","contents_url":"https://api.github.com/repos/broadinstitute/rawls/contents/{+path}","compare_url":"https://api.github.com/repos/broadinstitute/rawls/compare/{base}...{head}","merges_url":"https://api.github.com/repos/broadinstitute/rawls/merges","archive_url":"https://api.github.com/repos/broadinstitute/rawls/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/broadinstitute/rawls/downloads","issues_url":"https://api.github.com/repos/broadinstitute/rawls/issues{/number}","pulls_url":"https://api.github.com/repos/broadinstitute/rawls/pulls{/number}","milestones_url":"https://api.github.com/repos/broadinstitute/rawls/milestones{/number}","notifications_url":"https://api.github.com/repos/broadinstitute/rawls/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/broadinstitute/rawls/labels{/name}","releases_url":"https://api.github.com/repos/broadinstitute/rawls/releases{/id}","created_at":"2015-04-17T17:16:07Z","updated_at":"2015-06-12T15:10:03Z","pushed_at":"2015-08-31T15:44:30Z","git_url":"git://github.com/broadinstitute/rawls.git","ssh_url":"git@github.com:broadinstitute/rawls.git","clone_url":"https://github.com/broadinstitute/rawls.git","svn_url":"https://github.com/broadinstitute/rawls","homepage":null,"size":3124,"stargazers_count":3,"watchers_count":3,"language":"Scala","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":3,"forks":0,"open_issues":3,"watchers":3,"default_branch":"master"}},"base":{"label":"broadinstitute:master","ref":"master","sha":"333e1eefc48fc6f442e0e2ed08160ff317399e60","user":{"login":"broadinstitute","id":393552,"avatar_url":"https://avatars.githubusercontent.com/u/393552?v=3","gravatar_id":"","url":"https://api.github.com/users/broadinstitute","html_url":"https://github.com/broadinstitute","followers_url":"https://api.github.com/users/broadinstitute/followers","following_url":"https://api.github.com/users/broadinstitute/following{/other_user}","gists_url":"https://api.github.com/users/broadinstitute/gists{/gist_id}","starred_url":"https://api.github.com/users/broadinstitute/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/broadinstitute/subscriptions","organizations_url":"https://api.github.com/users/broadinstitute/orgs","repos_url":"https://api.github.com/users/broadinstitute/repos","events_url":"https://api.github.com/users/broadinstitute/events{/privacy}","received_events_url":"https://api.github.com/users/broadinstitute/received_events","type":"Organization","site_admin":false},"repo":{"id":34129770,"name":"rawls","full_name":"broadinstitute/rawls","owner":{"login":"broadinstitute","id":393552,"avatar_url":"https://avatars.githubusercontent.com/u/393552?v=3","gravatar_id":"","url":"https://api.github.com/users/broadinstitute","html_url":"https://github.com/broadinstitute","followers_url":"https://api.github.com/users/broadinstitute/followers","following_url":"https://api.github.com/users/broadinstitute/following{/other_user}","gists_url":"https://api.github.com/users/broadinstitute/gists{/gist_id}","starred_url":"https://api.github.com/users/broadinstitute/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/broadinstitute/subscriptions","organizations_url":"https://api.github.com/users/broadinstitute/orgs","repos_url":"https://api.github.com/users/broadinstitute/repos","events_url":"https://api.github.com/users/broadinstitute/events{/privacy}","received_events_url":"https://api.github.com/users/broadinstitute/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/broadinstitute/rawls","description":"Rawls service for DSDE","fork":false,"url":"https://api.github.com/repos/broadinstitute/rawls","forks_url":"https://api.github.com/repos/broadinstitute/rawls/forks","keys_url":"https://api.github.com/repos/broadinstitute/rawls/keys{/key_id}","collaborators_url":"https://api.github.com/repos/broadinstitute/rawls/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/broadinstitute/rawls/teams","hooks_url":"https://api.github.com/repos/broadinstitute/rawls/hooks","issue_events_url":"https://api.github.com/repos/broadinstitute/rawls/issues/events{/number}","events_url":"https://api.github.com/repos/broadinstitute/rawls/events","assignees_url":"https://api.github.com/repos/broadinstitute/rawls/assignees{/user}","branches_url":"https://api.github.com/repos/broadinstitute/rawls/branches{/branch}","tags_url":"https://api.github.com/repos/broadinstitute/rawls/tags","blobs_url":"https://api.github.com/repos/broadinstitute/rawls/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/broadinstitute/rawls/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/broadinstitute/rawls/git/refs{/sha}","trees_url":"https://api.github.com/repos/broadinstitute/rawls/git/trees{/sha}","statuses_url":"https://api.github.com/repos/broadinstitute/rawls/statuses/{sha}","languages_url":"https://api.github.com/repos/broadinstitute/rawls/languages","stargazers_url":"https://api.github.com/repos/broadinstitute/rawls/stargazers","contributors_url":"https://api.github.com/repos/broadinstitute/rawls/contributors","subscribers_url":"https://api.github.com/repos/broadinstitute/rawls/subscribers","subscription_url":"https://api.github.com/repos/broadinstitute/rawls/subscription","commits_url":"https://api.github.com/repos/broadinstitute/rawls/commits{/sha}","git_commits_url":"https://api.github.com/repos/broadinstitute/rawls/git/commits{/sha}","comments_url":"https://api.github.com/repos/broadinstitute/rawls/comments{/number}","issue_comment_url":"https://api.github.com/repos/broadinstitute/rawls/issues/comments{/number}","contents_url":"https://api.github.com/repos/broadinstitute/rawls/contents/{+path}","compare_url":"https://api.github.com/repos/broadinstitute/rawls/compare/{base}...{head}","merges_url":"https://api.github.com/repos/broadinstitute/rawls/merges","archive_url":"https://api.github.com/repos/broadinstitute/rawls/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/broadinstitute/rawls/downloads","issues_url":"https://api.github.com/repos/broadinstitute/rawls/issues{/number}","pulls_url":"https://api.github.com/repos/broadinstitute/rawls/pulls{/number}","milestones_url":"https://api.github.com/repos/broadinstitute/rawls/milestones{/number}","notifications_url":"https://api.github.com/repos/broadinstitute/rawls/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/broadinstitute/rawls/labels{/name}","releases_url":"https://api.github.com/repos/broadinstitute/rawls/releases{/id}","created_at":"2015-04-17T17:16:07Z","updated_at":"2015-06-12T15:10:03Z","pushed_at":"2015-08-31T15:44:30Z","git_url":"git://github.com/broadinstitute/rawls.git","ssh_url":"git@github.com:broadinstitute/rawls.git","clone_url":"https://github.com/broadinstitute/rawls.git","svn_url":"https://github.com/broadinstitute/rawls","homepage":null,"size":3124,"stargazers_count":3,"watchers_count":3,"language":"Scala","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":3,"forks":0,"open_issues":3,"watchers":3,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/broadinstitute/rawls/pulls/101"},"html":{"href":"https://github.com/broadinstitute/rawls/pull/101"},"issue":{"href":"https://api.github.com/repos/broadinstitute/rawls/issues/101"},"comments":{"href":"https://api.github.com/repos/broadinstitute/rawls/issues/101/comments"},"review_comments":{"href":"https://api.github.com/repos/broadinstitute/rawls/pulls/101/comments"},"review_comment":{"href":"https://api.github.com/repos/broadinstitute/rawls/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/broadinstitute/rawls/pulls/101/commits"},"statuses":{"href":"https://api.github.com/repos/broadinstitute/rawls/statuses/a7ce0f73ca16535953d02191ae12923472728077"}}}}