Conversation
|
Firstly, its not scalable idea to reuse services. I hope we don't have common Service types. You mentioned TemplateService is duplicated, and I believe it should be. |
|
Also , I believe, if we have dedicated packages already for |
they are "duplicated" because there are differences, i like to avoid "duplication", but do not have idea now how to do it, |
sure, i agree that it making more sense to have project related types in project related package, we can definitely discuss it in team and make some decision |
|
I used this tool previously (even wrote an sbt plugin to wrap it: https://github.com/coralogix/sbt-protodep): https://github.com/stormcat24/protodep which we probably could use to store the protos in separate repos and depend on each other. (I never used it in a way that one set of protos depended on another, just to share the protos between multiple service repos) |
|
Maybe instead of having two |
|
I also would do the refactoring of |
|
Regarding duplication of service interfaces, I think I like @justcoon 's approach more: to keep as much un-duplicated as possible and only split where necessary, instead of just duplicating everything and have two completely distinct set of proto files. So in general I like this PR |
|
For an OSS dev, the name TemplateService makes easier sense. So if we are having explicit names I guess I personally prefer that to be done in cloud repo. May be for closed source, I suggest CloudTemplateService (CloudWorkerService etc) so that IDE can list with all services that are part of Cloud. This idea can be applied to all services. I initially didn't change naming because I couldn't reach a conclusion on what to. Now it's time. |
|
Regarding moving data types to different packages, I'm OK with that, although I think there are some which does not necessarily belong to only a single service - those could remain in some shared place like now |
|
I added Cloud* prefix for grpc cloud services, |
Don't know if this creates lots of merge conflicts. Module imports and renamings during merge conflict resolution can be a super painful task. Not sure if we need renaming as part of this PR. But not strongly opinionated. cc @justcoon @vigoo @adamgfraser |
|
From an OSS development/contribution perspective, developer friendliness is something super important. If I want IDE to list all workerExecutuonError, then prefix is better than suffix Also suffix Error looks a bit overly verbose |
there are already changes anyway, now it can take more time to revert it here, then apply it in golem cloud ... |
# Conflicts: # golem-cloud-server-oss/src/grpcapi/mod.rs # golem-cloud-server-oss/src/grpcapi/template.rs
# Conflicts: # Cargo.lock # golem-worker-executor-oss/src/context.rs # golem-worker-executor-oss/tests/common/mod.rs
# Conflicts: # golem-cloud-server-oss/src/grpcapi/template.rs # golem-worker-executor-base/tests/common/mod.rs
|
Approved |
GOL-225 Update cargo component deps automatically
Feat/UI integration
GOL-225 Update cargo component deps automatically
CONTRIBUTING.md golemcloud#8 turned out to be unsatisfiable in the sandbox. The cli integration test suite has exactly two modules: - `bridge_gen::*` — generates wrapper code at test time, then `npm install` / `cargo build`s it. No pre-vendorable deps because the manifests don't exist until generation. - `app::*` — scaffolds mixed TS+Rust apps and runs `golem-cli build` on them; same dynamic-deps shape. Filtering both leaves zero tests, so the check would be vacuous (\"0 passed; 0 failed; 113 filtered out\"). Removed in favor of documenting that it's a local-only check. Everything else from CONTRIBUTING.md is wired.
3 participants
grpc api extracted from
golem-commonand moved/split togolem-api-grpcandgolem-api-grpc-cloudgolem.commonpackage, services and related types have own package, for exampleTemplateServicehavegolem.templatepackagegolem.cloudpackage, common types and structure are ingolem.cloud.commonpackage, services have own package and Cloud prefix, for exampleCloudProjectServicehavegolem.cloud.projectpackage,TemplateService-s (OSS TemplateService and CloudTemplateService), as there are differences in case ofProjectIdGolemErrorwas refactored togolem.worker.WorkerExecutionErrorTODO
golem.worker.WorkerExecutionErrorvariants have naming likeInvalidRequest,WorkerAlreadyExists..., which in protobuf namespace isgolem.worker.InvalidRequest,golem.worker.WorkerAlreadyExists, at this point this may easily make naming conflict, they should haveWorkerExecutionErrorin name, probably like:golem.worker.InvalidRequestWorkerExecutionError,golem.worker.WorkerAlreadyExistsWorkerExecutionErrorgolem-api-grpc-cloud- needgolem-api-grpcproto files for compilation, we need to figure out how we provide those dependencies whengolem-api-grpc-cloudwill be moved to cloud projectsAccountIdandResourceLimitsfromgolem-api-grpctogolem-api-grpc-cloud- now those are used in worker executor OSS impl., remove authorisation from worker executor OSS impl.