-
Notifications
You must be signed in to change notification settings - Fork 33
[STUD-490] add cluster api support #236
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
Merged
jmrog
merged 5 commits into
stardog-union:v2
from
rmyers:feature/STUD-490-add-cluster-api
Oct 21, 2020
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
2ce9d2a
[STUD-490] Add cluster api support
rmyers 6853cc7
[STUD-490] Removing done callback in spec files
rmyers 5197075
[STUD-490] Removing unused param and add docs
rmyers af8c880
[STUD-490] Adding docker-compose cluster test suite
rmyers 66fd329
[STUD-490] feature: add cluster support - address comments
rmyers File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| pack.enabled=true | ||
| pack.zookeeper.address=zoo1:2181 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| version: '2' | ||
| services: | ||
| stardog1: | ||
| command: ["--port", "5821", "--home", "/var/opt/stardog"] | ||
| depends_on: | ||
| - zoo1 | ||
| environment: | ||
| STARDOG_SERVER_JAVA_ARGS: -Xms2g -Xmx2g -XX:MaxDirectMemorySize=1g | ||
| THIS_HOST: stardog1 | ||
| THIS_PORT: 5821 | ||
| ZK_HOST_PORT: zoo1:2181 | ||
| image: stardog/stardog:latest | ||
| ports: | ||
| - 5821:5821 | ||
| restart: always | ||
| privileged: true | ||
| volumes: | ||
| - ../stardog-license-key.bin:/var/opt/stardog/stardog-license-key.bin | ||
| - ./cluster.properties:/var/opt/stardog/stardog.properties | ||
| stardog2: | ||
| command: ["--port", "5822", "--home", "/var/opt/stardog"] | ||
| depends_on: | ||
| - zoo1 | ||
| environment: | ||
| STARDOG_SERVER_JAVA_ARGS: -Xms2g -Xmx2g -XX:MaxDirectMemorySize=1g | ||
| THIS_HOST: stardog2 | ||
| THIS_PORT: 5822 | ||
| ZK_HOST_PORT: zoo1:2181 | ||
| image: stardog/stardog:latest | ||
| ports: | ||
| - 5822:5822 | ||
| restart: always | ||
| privileged: true | ||
| volumes: | ||
| - ../stardog-license-key.bin:/var/opt/stardog/stardog-license-key.bin | ||
| - ./cluster.properties:/var/opt/stardog/stardog.properties | ||
| zoo1: | ||
| environment: | ||
| ZOO_MY_ID: 1 | ||
| ZOO_SERVERS: server.1=zoo1:2888:3888 | ||
| image: zookeeper:3.4.14 | ||
| ports: | ||
| - 2181 | ||
| restart: always |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,4 +5,4 @@ node_modules/ | |
| dist/** | ||
| .vscode/ | ||
| .rpt2_cache/ | ||
|
|
||
| stardog-license-key.bin | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| const { fetch } = require('./fetch'); | ||
| const { httpBody } = require('./response-transforms'); | ||
|
|
||
| const info = conn => { | ||
| const headers = conn.headers(); | ||
| headers.set('Accept', 'application/json'); | ||
| return fetch(conn.request('admin', 'cluster'), { | ||
| headers, | ||
| }).then(httpBody); | ||
| }; | ||
|
|
||
| const status = conn => { | ||
| const headers = conn.headers(); | ||
| headers.set('Accept', 'application/json'); | ||
| return fetch(conn.request('admin', 'cluster', 'status'), { | ||
| headers, | ||
| }).then(httpBody); | ||
| }; | ||
|
|
||
| module.exports = { | ||
| info, | ||
| status, | ||
| }; | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| /* eslint-env jest */ | ||
|
|
||
| const { cluster } = require('../../lib'); | ||
| const { ConnectionFactory } = require('../setup-database'); | ||
|
|
||
| describe('cluster.info()', () => { | ||
| const coordinatorPort = 5821; | ||
| const participantPort = 5822; | ||
| const expectedNodeCount = 2; | ||
|
|
||
| it('should retrieve a JS object containing cluster information', () => { | ||
| const coordinatorConnection = ConnectionFactory(coordinatorPort); | ||
|
|
||
| return cluster.info(coordinatorConnection).then(res => { | ||
| const { nodes } = res.body; | ||
| const coordinator = nodes.find(node => node.includes(coordinatorPort)); | ||
| const participant = nodes.find(node => node.includes(participantPort)); | ||
|
|
||
| expect(nodes).toHaveLength(expectedNodeCount); | ||
| expect(coordinator).not.toBeUndefined(); | ||
| expect(participant).not.toBeUndefined(); | ||
| }); | ||
| }); | ||
|
|
||
| it('should respond correctly from both nodes', () => { | ||
| const participantConnection = ConnectionFactory(participantPort); | ||
|
|
||
| return cluster.info(participantConnection).then(res => { | ||
| const { nodes } = res.body; | ||
| const coordinator = nodes.find(node => node.includes(coordinatorPort)); | ||
| const participant = nodes.find(node => node.includes(participantPort)); | ||
|
|
||
| expect(nodes).toHaveLength(expectedNodeCount); | ||
| expect(coordinator).not.toBeUndefined(); | ||
| expect(participant).not.toBeUndefined(); | ||
| }); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| /* eslint-env jest */ | ||
|
|
||
| const { cluster } = require('../../lib'); | ||
| const { ConnectionFactory } = require('../setup-database'); | ||
|
|
||
| describe('cluster.status()', () => { | ||
| const coordinatorPort = 5821; | ||
| const participantPort = 5822; | ||
| const expectedNodeCount = 2; | ||
|
|
||
| it('should retrieve a JS object containing cluster status', () => { | ||
| const coordinatorConnection = ConnectionFactory(coordinatorPort); | ||
|
|
||
| return cluster.status(coordinatorConnection).then(res => { | ||
| const { nodes } = res.body; | ||
| const coordinator = nodes.find(node => | ||
| node.address.includes(coordinatorPort) | ||
| ); | ||
| const participant = nodes.find(node => | ||
| node.address.includes(participantPort) | ||
| ); | ||
|
|
||
| expect(nodes).toHaveLength(expectedNodeCount); | ||
| expect(coordinator).not.toBeUndefined(); | ||
| expect(coordinator.metadata).not.toBeUndefined(); | ||
| expect(coordinator.type).toEqual('FULL'); | ||
| expect(participant).not.toBeUndefined(); | ||
| expect(participant.metadata).not.toBeUndefined(); | ||
| expect(participant.type).toEqual('FULL'); | ||
| }); | ||
| }); | ||
|
|
||
| it('should respond correctly from both nodes', () => { | ||
| const participantConnection = ConnectionFactory(participantPort); | ||
|
|
||
| return cluster.status(participantConnection).then(res => { | ||
| const { nodes } = res.body; | ||
| const coordinator = nodes.find(node => | ||
| node.address.includes(coordinatorPort) | ||
| ); | ||
| const participant = nodes.find(node => | ||
| node.address.includes(participantPort) | ||
| ); | ||
|
|
||
| expect(nodes).toHaveLength(expectedNodeCount); | ||
| expect(coordinator).not.toBeUndefined(); | ||
| expect(coordinator.metadata).not.toBeUndefined(); | ||
| expect(coordinator.type).toEqual('FULL'); | ||
| expect(participant).not.toBeUndefined(); | ||
| expect(participant.metadata).not.toBeUndefined(); | ||
| expect(participant.type).toEqual('FULL'); | ||
| }); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but a few general (somewhat nitpicky) comments here:
Instead of requiring
Fetchand doingFetch.fetch, the convention in stardog.js is to doconst { fetch } = require('./fetch'). Arguably, the former is cleaner because it doesn't shadow thefetchglobal variable in the browser and makes it explicit that you are calling a differentfetch, but this is a small matter and I'd prefer sticking to convention for now.You can leave out the
paramsparameter when it's not being used. Here, the stardog.js codebase is simply inconsistent right now -- sometimes theparamsparameter is included when it's not being used, and sometimes it isn't -- but we're trying to move toward "only include it when it's actually being used."There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed the import to make it possible to mock the fetch function. Since it needs an Object to spyOn, couldn't get it to work with a direct import. But I'm working on getting the cluster to run with docker compose and then I can change the tests to a full integration instead of mocks to address this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was able to get a simple cluster to run and have updated the tests to use the cluster nodes for the connection without mocking fetch. This should address the code style issues.