-
Notifications
You must be signed in to change notification settings - Fork 234
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
[ATO-2731] Initialise gRPC health check service #1129
Conversation
a712bea
to
be82714
Compare
be82714
to
14b925b
Compare
|
||
return grpc.insecure_channel( | ||
target=f"{GRPC_HOST}:{GRPC_NO_TLS_PORT}", | ||
compression=grpc.Compression.Gzip) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the arguments be all in one line? Wondering how this passed the Code Quality 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They would go beyond 88 character line limit if all arguments were in one line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Running ruff format
for this code formats it as
return grpc.insecure_channel(
target=f"{GRPC_HOST}:{GRPC_NO_TLS_PORT}", compression=grpc.Compression.Gzip
)
with the line length of 84.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick anyways. I know if this was rasa-private
that the Code Quality would fail for this. Just raising it for the concern if the Code Quality check is proper for rasa-sdk
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, yes, but we are using black
as formatter in Rasa SDK ATM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a PR to switch to ruff and remove black alltogether.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very clean CI 💯
* Initialize the built-in gRPC health service * Do not consider deleted files for filtering * Remove comment which indicates that grpc code-in-sync check is not run on CI * Prefix private functions from gRPC server module with underscore * Add integration tests for gRPC * Add changelog for healthcheck service in gRPC mode
* Initialize the built-in gRPC health service * Do not consider deleted files for filtering * Remove comment which indicates that grpc code-in-sync check is not run on CI * Prefix private functions from gRPC server module with underscore * Add integration tests for gRPC * Add changelog for healthcheck service in gRPC mode
Makefile
Added new target in Makefile which will build docker image with dev dependencies.
Initialise gRPC health check service
Health check for gRPC server is initialised.
Added Integration tests for gRPC server:
Added CI steps which will run integration tests, using targets from Makefile.
Generate SSL certificates
Added
Makefile
with targets to generate SSL certificates, located undercerts
directory.Generated certificates will be placed in
certs
directory because that will prevent accidental commit of the certificates (seecert/.gitignore
).Move generated certificates and keys you need, to the directory of integration test for which they will be used.
Status (please check what you already did):
black
(please check Readme for instructions)