feat(microservices): expose transport server getter#17028
Open
OlegStrokan wants to merge 1 commit into
Open
Conversation
Contributor
Author
a817ce8 to
56b1105
Compare
Coverage Report for CI Build 0Coverage decreased (-0.08%) to 89.907%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions32 previously-covered lines in 2 files lost coverage.
Coverage Stats
💛 - Coveralls |
Member
|
could you please target v12.0.0 branch? |
083b78a to
2bc7b90
Compare
Contributor
Author
done. the pipeline failed. I will check whether its related to my changes EDIT: pipeline fixed |
2bc7b90 to
4063cdb
Compare
Contributor
Author
|
can someone review/approve this PR? It would be nice to have this feature in nest v12 |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.

PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Currently
INestMicroservicehave no way to stop only the transport layer (port/connection) without triggering the full application shutdown lifecycle, when i callclose()method fromINestMicroservicestops the transport and triggers the full shutdown lifecycle - which is correct, but my business case is specific: the server has more then one grpc connection: control port (receiving commands), data port (for streaming data). when control sends a new TLS value the component needs to stop only the data port and restart it with new TLS value. Other port should remain active. butmicroservice.close()tears down everything because lifecycle hooks fire across entire di containerIssue Number: N/A
What is the new behavior?
Now we can stop listening on the transport layer, no lifecycle hooks, di side effects, so app stays alive
example of using:
await microservice.getTransportServer().close()and of course old one:
await microservice().close()- still shut down everythingDoes this PR introduce a breaking change?
Other information
i am not ai, i am a real person. the code was tested manually injecting updated nestjs into my infra + runnig tests.
i am aware of nestjs design - what microservice transport start once and die with the app, BUT i have unusual case where my application is a industrial adapter which needs process tls hot swap without shutdowning because there are another clients which depends on this application. I could probably bypass it using raw gprc lib, but i think it's better solution (also simple one)
i created
ITransportServerto prevent circular deps between common and microservices packages. It's second version of my code implementation. In first one i just used "any" without adding new type, but later when i really injected new nest into my services it was inconvenient to use. I am open to discuss about different implementationin the last commit i marked the abstract Server class as
implements ITransportServertoenforce the contract at compile time. ts structural typing already handle Server and ITransportServer compatibility , but without the explicit declaration a some new future changes to server.close() or server.listen() could silently break the interface guarantee withou compiler error at the source