introducing new api package

[ilgooz]

resolves #377, #374.
depends on #371, so it needed to be merged first. This PR will be rebased to dev after #371 made it's way through dev.

note -1:
currently, writing unit tests for api package is a bit hacky or not passible in some cases. so they'll be added later when we refactor depended packages like service, execution and pubsub etc so we can mock/fake them.

[ilgooz]

needs review

[ilgooz]

needs review. it'd be much simpler if you review this PR per commit. since we move old api folder to interface/grpc you'll not be able to see diff on each file if you review from total view. But if you review by commits you'll see each single diffs including renamed files and their further changes in the next commits.

[antho1404]

Service test folder

We have services for tests in different packages. I like the idea to centralize these tests in on place but let's do that for all of them or none of them. We have more in cmd/service/test, service/importer/test Also I would recommend to do that in another PR.

API Package

I find a bit complicated to have in the api these kind of delete.go and delete_deleter.go and this for every api. I don't like the part that we have a specific type for each feature eg: serviceDeleter that are always the same with the api *API

Feature types with options

For example in the api.ListenResult function we have the options given to newResultListener. These options can be given to the actual listen function (same for any listen functions) and like that we can have a single type with only the api as attribute

[NicolasMahe]

I agree with Anthony,

It feel like you go halfway from no types in one package to many types in many packages.
I suggest to keep one type per package and a few packages for any group on function (could be: one package for service, one event, etc...)

[ilgooz]

@antho1404

1-

having a specific type for each feature is done to isolate functions from each other. you can see this kind of implementation detail in some of the standard packages too (there are more but i don't have the time to do a code research for now).

this kinda use is specially ideal when we expose some package level functions (APIs) and let them to return basic types and some model types (e.g. *service.Service) instead of returning doER types (these private structs we have) and also keep the isolation between API functions.

and if we don't use different types for each feature and create one struct instead, then we'll a have generic/giant struct type that does everything which is a poor way to organize this kind of package, not good for testing and we'll have possible collapses in the function names.

2-

about the options, I understand your concern. but i followed a pattern that where we have multiple methods that does the same job with a slight difference and receives the same options. like 1 and 2.

In this case i like to pass options to initializer function and it'll iterate options one time and set it to struct fields. This way we don't need to write the same code for each function.

conclusion

I understand your concern about this, but lets not forget that we're composing almost all of the internal packages in this new api package. api package is a general purpose package unlike a single purpose package like service, execution, container etc. Being a general purpose package can easily result with a messy code if we don't apply necessary isolations between features. I like to hear more suggestions about this.

[ilgooz]

@NicolasMahe separating api package to child packages can be nice but might complicate things a bit, currently we don't have 10s of API methods so this version is also pretty maintainable if we keep the isolation. I think we can switch to child packages in future, when we think it's needed and in another PR. Happy to hear more suggestions about this.

[ilgooz]

@antho1404
We have services for tests in different packages. I like the idea to centralize these tests in on place but let's do that for all of them or none of them. We have more in cmd/service/test, service/importer/test Also I would recommend to do that in another PR.

I agree to this except we should keep test services for importer in it's own context. Because importer is a parser, it might need lots of test services to test syntax errors etc. and the test services it uses will not needed to be used to test things in other packages.