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

Discovery service based on proto endpoint #30

Merged
merged 8 commits into from
Nov 16, 2017

Conversation

calvellido
Copy link
Contributor

  • Add fetch-proto script

    • Add proto files fetching node script
    • Update README with specifics on how to fetch proto files
    • Add axios dependency to be able make HTTP requests
    • Add metrics proto file to proto directory
  • Use google-protobuf JS runtime

    • Update gen-proto-code script to use protoc binary.
    • Update npm scripts.
    • Update README.
    • Update dependencies
  • Fix error message on gen-proto-code script

  • Add JS generated classes from proto descriptors

  • Add microservices discovery service

    • Add a microservices information fetching method, deserializing a meesaged based on the proto associated with that endpoint.
    • Consume that service on the app main component.
    • Set an environment variable that points to that microservices and nodes discovery information.

This closes #13

* Add try-catch block to control exceptions on run function
* Change proto generation script spinner appearance for a better look
* Add proto files fetching node script
* Update README with specifics on how to fetch proto files
* Add axios dependency to be able make HTTP requests
* Add metrics proto file to proto directory
* Update gen-proto-code script to use protoc binary.
* Update npm scripts.
* Update README.
* Update dependencies
* Add a microservices information fetching method, deserializing a meesaged based on the proto associated with that endpoint.
* Consume that service on the app main component.
* Set an environment variable that points to that microservices and nodes discovery information.

This closes #13.
Copy link

@anamariamv anamariamv left a comment

Choose a reason for hiding this comment

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

Minor comments.

const urlObject = new URL(arg);
return urlObject;
} catch (e) {
// throw new Error(`Error: ${arg} is not a valid URL path`);

Choose a reason for hiding this comment

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

Remove comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adressed on 59ef685

})
.catch((error) => {
spinner.fail(`Error connecting to: ${error.config.url}`);
// shell.echo(`Error connecting to: ${error.config.url}`);

Choose a reason for hiding this comment

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

same above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adressed on 59ef685

@@ -1,5 +1,6 @@
export const environment = {
production: true,
// metricsEndpoint: 'ws://localhost:8080/metrics'
metricsEndpoint: 'ws://localhost:3000'
metricsEndpoint: 'ws://localhost:3000',

Choose a reason for hiding this comment

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

Comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adressed on 59ef685

@@ -6,5 +6,6 @@
export const environment = {
production: false,
// metricsEndpoint: 'ws://localhost:8080/metrics'
metricsEndpoint: 'ws://localhost:3000'
metricsEndpoint: 'ws://localhost:3000',

Choose a reason for hiding this comment

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

Same above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adressed on 59ef685

@calvellido calvellido merged commit 9f70e17 into master Nov 16, 2017
@calvellido calvellido deleted the jv-13-discovery-service branch November 16, 2017 14:38
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.

Discovery service implementation
2 participants