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

[Microservice] - Create two microservices #1

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

endouakulu
Copy link
Owner

No description provided.

@endouakulu endouakulu added the WIP label Nov 5, 2017
Repository owner deleted a comment Nov 5, 2017
Repository owner deleted a comment Nov 5, 2017
Repository owner deleted a comment Nov 5, 2017
Repository owner deleted a comment Nov 5, 2017
Repository owner deleted a comment Nov 5, 2017
Copy link
Collaborator

@salmanebah salmanebah left a comment

Choose a reason for hiding this comment

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

Because the settings microservice is not as simple as the currency one. We can split the responsibility in different classes, something like: Handler -> Service -> Repository

<dependency>
<groupId>io.vertx</groupId>
<artifactId>vertx-web-client</artifactId>
<version>3.5.0</version>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Using maven dependencyManagement would help for future version upgrade

<dependency>
<groupId>ch.qos.logback</groupId>
<artifactId>logback-classic</artifactId>
<version>1.1.5</version>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as the previous comment

</testResource>
</testResources>

<plugins>
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can maybe pull this up in the parent to share the plugin configurations between currency and setting modules

public class CurrencyMicroservice extends AbstractVerticle {

public static final String APPLICATION_JSON_CHARSET_UTF_8 = "application/json; charset=utf-8";
private static Logger logger = LoggerFactory.getLogger(CurrencyMicroservice.class);
Copy link
Collaborator

Choose a reason for hiding this comment

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

logger can be made final

@Override
public void start(Future<Void> startFuture) throws Exception {
super.start(startFuture);
System.setProperty(LoggerFactory.LOGGER_DELEGATE_FACTORY_CLASS_NAME, SLF4JLogDelegateFactory.class.getName());
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can set this property at the time of running the app or put it in the pom maybe ?

HttpResponse<Buffer> response = ar.result();
logger.info(response.bodyAsString());
routingContext.response()
.setStatusCode(200)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Noob question: vertx doesn't provide a StatusCode type ?

Copy link
Owner Author

Choose a reason for hiding this comment

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

I don't think so. I have to check it

super.start(startFuture);
MetricsService metricsService = MetricsService.create(vertx);
HttpServer server = vertx.createHttpServer();
JDBCClient client = JDBCClient.createShared(vertx, new JsonObject()
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can externalize this in a configuration file

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yes. There is a feature in vertx for configuration management

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants