Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

@krysalead
Copy link

Hi,

Sorry for the formatting it seems that some space were missing in front of the comments and I don't know how to remove this indentation from Visual Studio Code.
Changes are not that big, only a new config key
serviceProtocol which could be http or https (default is http)
The other change is that if the service was registered as https it will be called with https.

@krysalead
Copy link
Author

Hi,

I had to change the eslintrc to make it work from command line, tell me if it is breaking on your side.
I am using VS code

@cjus cjus self-assigned this Jul 30, 2018
@krysalead
Copy link
Author

Documentation

HTTPS

If you build your microservice not behing a firewall but using different hosting service you will like to run it in HTTPS for security reason. This is doable in Hydra as simply as setting the service protocol to https.

{
    "serviceName": "echo-service",
    "serviceIP": "127.0.0.1",
    "servicePort": 3000,
    "serviceType": "",
    "serviceDescription": "",
    "serviceProtocol": "https",
    "redis": {
      "url": "127.0.0.1",
      "port": 6379,
      "db": 1
    }
  }
}

Your service will be registered in redis as secured and will be called using HTTPS. Nothing additional to do in your call to the service.

If you are not using a revers-proxy handeling the SSL encryption you will need to use hydra-express for instance that is starting a server in HTTPS if you specify https in protocol see hydra-express documentation for more information.

@cjus
Copy link
Contributor

cjus commented Jul 31, 2018

@krysalead thanks for this PR. In reviewing it there seems to be a lot of formatting changes. I do agree that those format changes should be done - but I'd rather do that in another PR. Can you branch and only apply your core changes in a new PR?

@krysalead
Copy link
Author

Hi,

I am really struggling to make it properly, there is inconsistency everywhere, spaces sometimes added, sometimes not. Isn't simpler to do the review with comparison without white space activated?

Copy link
Contributor

@cjus cjus left a comment

Choose a reason for hiding this comment

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

Question and minor feedback

.eslintrc Outdated
{
"plugins": ["mocha"],
"extends": ["eslint:recommended", "google"],
"extends": ["eslint:recommended", "./node_modules/eslint-config-google/index.js"],
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this necessary?

Copy link
Author

Choose a reason for hiding this comment

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

It was not loading google eslint config in my environement, I manage to make it work like that but I can revert it.

index.js Outdated
host: instance.ip,
port: instance.port,
path: parsedRoute.apiRoute,
protocol: (instance.protocol || 'http') + ':',
Copy link
Contributor

Choose a reason for hiding this comment

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

or: protocol: ${(instance.protocol || 'http')}:,

Copy link
Author

Choose a reason for hiding this comment

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

If you prefer, I am still not use to litterals on small piece of string.

@cjus
Copy link
Contributor

cjus commented Aug 1, 2018

@krysalead No worries about the spaces - I can cleanup after if necessary. See my questions and comment. Thanks again.

@JustusFluegel
Copy link

JustusFluegel commented Nov 22, 2020

maybe consider using the modern ?? syntax instead of || like a ?? "default"

see https://dev.to/hereisnaman/logical-or-vs-nullish-coalescing-operator-in-javascript-3851 for details

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.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.