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

[MODAUD-195]. Implement consumer & endpoint for invoice records #175

Merged
merged 9 commits into from
Nov 4, 2024

Conversation

BKadirkhodjaev
Copy link
Contributor

@BKadirkhodjaev BKadirkhodjaev commented Oct 31, 2024

Purpose

Approach

  • Add a Kafka consumer handler and verticle to consume an Invoice event
  • Add associated REST API, service and DAO classes
  • Add necessary descriptors, API and SQL code generation
  • Cover with unit tests and some manual Kafka CLI and Postman testing
  • Replace deprecated worker verticle deployment option syntax with the newer one at several places

@BKadirkhodjaev BKadirkhodjaev marked this pull request as ready for review October 31, 2024 13:18
@BKadirkhodjaev BKadirkhodjaev requested review from gurleenkaurbp and a team October 31, 2024 13:18
new DeploymentOptions()
.setWorker(true)
.setInstances(acqPieceConsumerInstancesNumber), pieceEventsConsumer);
deployVerticle(vertx, verticleFactory, OrderEventConsumersVerticle.class, acqOrderConsumerInstancesNumber, acqOrderConsumerPoolSize, orderEventsConsumer);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

String query = format(GET_BY_INVOICE_ID_SQL, logTable, logTable, format(ORDER_BY_PATTERN, sortBy, sortInvoice));
Tuple queryParams = Tuple.of(UUID.fromString(invoiceId), limit, offset);
pgClientFactory.createInstance(tenantId).selectRead(query, queryParams, promise);
} catch (Exception e) {
Copy link
Contributor

Choose a reason for hiding this comment

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

catch is not needed here, in case of failures we will have failed future.
Potentailly it can be a chance of runtime excpeptions even before creating futures, but it is very low I think. And if there is for example some DB failure - this catch will not help and if you want to log - .onFailure can be used

"javaType": "java.lang.Object"
}
},
"additionalProperties": false
Copy link
Contributor

Choose a reason for hiding this comment

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

if this even structure changes in the futures, this dto also have to changes because of "additionalProperties": false.
this field don't allow extra property in event. default true (if additionalProperties is not exists)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if this even structure changes in the futures, this dto also have to changes because of "additionalProperties": false. this field don't allow extra property in event. default true (if additionalProperties is not exists)

So you recommend to set it to true?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes

Copy link

sonarqubecloud bot commented Nov 4, 2024

@BKadirkhodjaev BKadirkhodjaev merged commit 3829790 into master Nov 4, 2024
6 checks passed
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