Configure services with envs #641
Conversation
krhubert
force-pushed
the
feature/passing-env
branch
from
eb668b4
to
434c1c1
Compare
I agree, seeing the dev, it adds a lot of complexity for nothing. I don't see any sufficient reasons to keep that. configuration:
env:
- FOO=barThis is the same as docker compose https://docs.docker.com/compose/environment-variables/ |
|
This pull request has been mentioned on MESG Community. There might be relevant details there: https://forum.mesg.com/t/configuring-services-with-deploy-time-or-run-time-arguments/124/11 |
| @@ -244,6 +250,7 @@ message DeployServiceRequest { | |||
| string url = 2; // Git repo url of service. When url provided, stream will be closed after the first receive. | |||
| bytes chunk = 3; // Chunks of gzipped tar archive of service. If chunk provided, stream should be closed by client after all chunks sent. | |||
| } | |||
| map<string, string> env = 4; // Env used to deploy service. | |||
There was a problem hiding this comment.
needs to be inside oneOf since it's not sent in every message.
There was a problem hiding this comment.
Why, can you explain more?
I don't want to put it inside one of as it's not one of option to deploy, but a separate parameter
There was a problem hiding this comment.
onOf syntax does not stand for optional values in gRPC. We need it exactly for the same reason explained in the docs. https://developers.google.com/protocol-buffers/docs/proto3#oneof. env variables needs to be in the oneOf section, they're not sent in the every message of the stream.
There was a problem hiding this comment.
solved by 59b574f:
deploy api now has 3 fields(inside oneOf):
- options
- url
- chunk
these fields should be sent to service in the following order.
- options must be sent as 1th(not required to send when options aren't provided).
- url or chunks must be sent as 2nd (if chunks chosen, client should close the stream after all chunks sent) or as 1th when options aren't provided.
this flow is much easier to implement by the server and gRPC clients(our consumers).
|
I recommend merging this PR first #642. It'll be used by this PR. |
krhubert
force-pushed
the
feature/passing-env
branch
3 times, most recently
from
2e458e9
to
385671d
Compare
krhubert
force-pushed
the
feature/passing-env
branch
from
385671d
to
62934d5
Compare
|
I've applied all the requested changes in a new PR #654. We should merge it to this one before continuing. |
|
@ilgooz I would like to merge this PR to dev as it is and improve it with multiple small PRs. |
|
This pull request has been mentioned on MESG Community. There might be relevant details there: https://forum.mesg.com/t/configuring-services-with-deploy-time-or-run-time-arguments/124/15 |
|
This pull request has been mentioned on MESG Community. There might be relevant details there: https://forum.mesg.com/t/configuring-services-with-deploy-time-or-run-time-arguments/124/17 |
No description provided.