Adding boilerplate code to easily migrate grpc endpoints #39
Conversation
shravanshetty1
changed the title
shravanshetty1
changed the title
shravanshetty1
changed the title
This reverts commit edaeeef.
shravanshetty1
changed the title
|
closes #49 |
| @@ -0,0 +1,101 @@ | |||
| // Code generated by protoc-gen-go-grpc. DO NOT EDIT. | |||
|
|
|||
There was a problem hiding this comment.
It is often considered bad to keep generated files under version control. So do you need these generated files under source control?
- Maybe you could add a script to generate them, and a section in the readme.md to explain how to generate them?
Points specific to our project.
- Users are encouraged to use docker scripts to run blobbers so it would be easy to add as an extra stage step in a dockerfile.
- 0chain modules are difficult to compile for the first time on a blank OS. So there is no expectation of running the
blobbermodule without preparation; which is one of the main factors in favour of keeping generated files in Github.
Anyway, I could go either way, just interested in hearing the reasons for keeping the generated code under source control?.
There was a problem hiding this comment.
Mostly for developer-experience, just like alot of other golang projects i have seen.
Generating at compile time will prevent any of the generated files from being out of date. I suggest we add github CI check to check if generated files are out of date and also regenerate the files while the service is being created in docker. This way any cons of keeping generated files in the codebase are eliminated, if there are other cons - please let me know.
Generated files could also fail in languages like c++ since the generated code will be OS specific. But since we are using golang this shouldnt be a problem since it has a runtime.
If you disagree with this i can remove the generated files as requested.
There was a problem hiding this comment.
I think it fine if, as you suggested, we can ensure that the generated files are kept up to date.
Maybe add a .github/workfolw so that before merging the generated files are updated and confirmed to work.
I think generated files merged into master should not be trusted to developers, but auto-generated by CI.
|
lets handle further changes in different issues since these PR's are getting too large |
| - "505${BLOBBER}:505${BLOBBER}" | ||
| command: ./bin/blobber --port 505${BLOBBER} --hostname localhost --deployment_mode 0 --keys_file keysconfig/b0bnode${BLOBBER}_keys.txt --files_dir /blobber/files --log_dir /blobber/log --db_dir /blobber/data --minio_file keysconfig/minio_config.txt | ||
| - "703${BLOBBER}:703${BLOBBER}" | ||
| command: ./bin/blobber --port 505${BLOBBER} --grpc_port 703${BLOBBER} --hostname localhost --deployment_mode 0 --keys_file keysconfig/b0bnode${BLOBBER}_keys.txt --files_dir /blobber/files --log_dir /blobber/log --db_dir /blobber/data --minio_file keysconfig/minio_config.txt |
There was a problem hiding this comment.
@anish-squareops A new arguement grpc_port has been added. Please confirm that this wont cause any issue. All changes are in docker.local. I am not sure if docker.local is being used in production.
closes #49
This PR introduces changes which integrates grpc into the current project. The changes include, basic grpc related setup code and 1 migrated endpoint
GetAllocation.