Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Refactor AT services #461

Merged
merged 5 commits into from
Nov 25, 2021
Merged

Refactor AT services #461

merged 5 commits into from
Nov 25, 2021

Conversation

Lagovas
Copy link
Collaborator

@Lagovas Lagovas commented Nov 22, 2021

There are nothing changed in features or public API. Only internal structures and implementations.
All shared components have moved to AcraTranslatorData structure that used to initialize services (HTTP, gRPC) and passed to callbacks that called on server initialization.

Refactored initialization dependencies like poison record callbacks, tokenizer and moved them from redundant factories to program startup to initialize them at start and share.

Added registries that store callbacks for events from gRPC/HTTP servers. It helps extend implementation with build flags. Also, updates integration tests with new env variable that accept and pass build flags for compilation commands.

Updated go_package option in .proto file to full path to fix import in other .proto files

And renamed TestPoisonRecordWholeCellStatusOff (that exists in 2 examples) with proper suffix.

Checklist

  share all data using only AcraTranslatorData struct
* pass build flags in integrations tests
* fix overriding test in integrations tests
* make getClientID as separate function instead of method of TLSServiceWrapper
// OngRPCServerInit call all registered callbacks on gRPC server initialization
func OngRPCServerInit(server *grpc.Server, data *common.TranslatorData, service DecryptService) {
lock.Lock()
defer lock.Unlock()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need to use defer here? If I am not mistaken the OnServerInit function does not return any error so here no possible early-exit scenarios, right? Could we place it after the loop statement?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Defer has used because of subscribers may panic. Yes, the program will interrupt and exit if nothing catches this panic. But for a future better to handle it from the start correctly.

@ZhmakAS ZhmakAS self-requested a review November 24, 2021 09:22
Copy link
Contributor

@ZhmakAS ZhmakAS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

update golang services according to updates grpc/grpc-go#3669
update Makefile with up-to-date building commands
@Lagovas Lagovas merged commit eee9eb4 into master Nov 25, 2021
@Lagovas Lagovas deleted the lagovas/AT-refactoring-for-extensions branch November 25, 2021 16:01
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants