Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New exchange search API to return how many nodes a particular service is currently running on #219

Merged
merged 6 commits into from
Sep 14, 2019

Conversation

sf2ne
Copy link
Collaborator

@sf2ne sf2ne commented Sep 14, 2019

This PR references:

This PR includes:

  • new search route for finding the list of nodes that a service is running on
  • add runningServices field to status resource in nodes
  • build value of runningServices on every PUT
  • add db update in Schema.scala
  • add pii translation files to resources
  • unit tests
  • unit test fix for the nodes error search route

val orgService = "%|"+orgid+"/"+service+"|%"
logger.info("SADIYAH: orgService: " + orgService)
val q = for {
(n, s) <- (NodesTQ.rows.filter(_.orgid === orgid)) join (NodeStatusTQ.rows.filter(_.runningServices like orgService)) on (_.id === _.nodeId)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For future reference, be aware that if there were more than 1 status resource for a particular node id, then node id would be repeated in your yield (and then you would have to remove duplicates before sending them to the client). In this case you are safe, because there can only ever be at most 1 status resource per node.

assert(postResp.nodes.head === "NodesSuiteTests/n1")
assert(postResp.nodes(1) === "NodesSuiteTests/n2")
assert(postResp.nodes.contains("NodesSuiteTests/n1"))
assert(postResp.nodes.contains("NodesSuiteTests/n2"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx for fixing this. I actually hit this once yesterday when running the test suites, but didn't have time to investigate, and reran the test suites and then everything worked (because it randomly depended on what order they came back in), so i moved on. But it left a nagging concern, but now that concern is gone :)

Copy link
Member

@bmpotter bmpotter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See 2 informational comments in the Files Changed section...

@bmpotter bmpotter merged commit 79c1af2 into open-horizon:master Sep 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants