Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Repository Separation in terms of Module #415

Open
tiokim opened this issue Nov 3, 2021 · 22 comments
Open

Repository Separation in terms of Module #415

tiokim opened this issue Nov 3, 2021 · 22 comments
Assignees
Labels
refactoring Any tasks and issues w.r.t. the code refactoring v2 v2.0.0

Comments

@tiokim
Copy link
Contributor

tiokim commented Nov 3, 2021

Home Edge is made up of several modules, and it would be good to run them independently as a microservice concept to minimize damage among modules.
image

  • Necessity

    • If Data Storage module affected by edgex imports a library like mqtt, Edge Orchestration will have to spend time importing the library. Maintaining DataStorage separately can save a lot of time in terms of Edge Orchestration, because Datastorage can use Edgex's Dockerfile.
    • Edge orchestration can avoid unintended side effects from edgex code like runtime error.
  • Suggestion

    • Edge Orchestration : lf-edge/edge-home-orchestration-go
    • Data Storage : lf-edge/edge-home-datastorage-go
@tiokim tiokim added the refactoring Any tasks and issues w.r.t. the code refactoring label Nov 3, 2021
@MoonkiHong
Copy link
Contributor

This is a wonderful suggestion and we have to think of how we could make it. Any thoughts? @NiranjanPatil25 @suresh-lc @tdrozdovsky?

@tdrozdovsky
Copy link
Contributor

I support this proposal.
I have only a question (for a very long time) why the project name contains -go part?

@suresh-lc
Copy link
Contributor

Ya had this thought long time ago, that time there weren't big modules as such. But now as we should see if we make them as microservices. Something like

  1. Core-Service - the basic bread and butter of the project (Discovery/Scoring/Offloading)
  2. Data-Service - Storing the data (EdgeX container)
  3. Cloud-Service - Interface to Cloud system
  4. Machine Learning

The current data storage can be made independent microservice and we can work on that. The cloud service which we plan to make better can be designed so.

@suresh-lc
Copy link
Contributor

I support this proposal. I have only a question (for a very long time) why the project name contains -go part?

Ya '-go' might not be needed for now. When start supporting different languages we can think of adding it.(Like how EdgeX does - C/Go)

@tiokim tiokim added the v2 v2.0.0 label Nov 8, 2021
@tiokim
Copy link
Contributor Author

tiokim commented Nov 8, 2021

@lf-edge/edge-home-orchestration-go-maintainers I've proposed lf-edge/edge-home-{ module }-go based on the previous repository name. Please share your good ideas, and make a good naming convention in terms of repository upon this opportunity. 😊

@tdrozdovsky
Copy link
Contributor

tdrozdovsky commented Nov 8, 2021

I support this idea (lf-edge/edge-home-{ module } ), but I think that specifying a programming language is bad practice (the github analyzes and displays programming languages, see pictures below).

изображение

and

изображение

@MoonkiHong
Copy link
Contributor

@NiranjanPatil25 @suresh-lc What about creating lf-edge/edge-home-datastorage first? Will wait for it. 😁

@nitu-s-gupta
Copy link
Contributor

Hello, To make datastorage independent module, I have started the work of cloning the repo of edge orchestration and removing the unnecessary code in terms of datastorage. But as we know few packages like logmgr, networkhelper, dbhelper are being used by storage manager. So as per Golang architecture restrictions, packages mentioned inside the internal package cannot be imported in the independent module. So for the same I have come up with two solutions,

  1. Copy the folders required by datastorage under the lfedge/edge-home-datastorage package i.e. local to datastorage
  2. Move the packages required by the datastorage out of internal folder and create a new folder pkg in edge-home-orchestration and import these packages in datastorage module as
    import https://github.com/lf-edge/edge-home-orchestration-go/pkg/logmgr and so on

Personally I would prefer the second option of moving the packages needed by another module to pkg folder to make it visible for other projects also for import

@MoonkiHong
Copy link
Contributor

@nitu-s-gupta This is great. Thank you for your suggestion. I love the option 2. 😁

@tiokim
Copy link
Contributor Author

tiokim commented Feb 15, 2022

Hello, To make datastorage independent module, I have started the work of cloning the repo of edge orchestration and removing the unnecessary code in terms of datastorage. But as we know few packages like logmgr, networkhelper, dbhelper are being used by storage manager. So as per Golang architecture restrictions, packages mentioned inside the internal package cannot be imported in the independent module. So for the same I have come up with two solutions,

  1. Copy the folders required by datastorage under the lfedge/edge-home-datastorage package i.e. local to datastorage
  2. Move the packages required by the datastorage out of internal folder and create a new folder pkg in edge-home-orchestration and import these packages in datastorage module as
    import https://github.com/lf-edge/edge-home-orchestration-go/pkg/logmgr and so on

Personally I would prefer the second option of moving the packages needed by another module to pkg folder to make it visible for other projects also for import

I think new datastorage container can not access to edge-orchestration db and it would be great if new datastorage is independent from edge-orchestration in terms of source code. Regarding logmgr, it would be better to use edgex logger since we have an issue.
I think we can think about as belows.

  1. make a rest api for device ip instead of using networkhelper.
  2. new datastorage reads device id from the file instead of using dbhelper.
  3. implement or copy the source code for others.

@nitu-s-gupta
Copy link
Contributor

Now there are votes for both option. Can we conclude for one option. Other members can also give their opinion

@nitu-s-gupta
Copy link
Contributor

Exposing as library by moving the folders to pkg folder for each functionality would help its usage going forward when microservice architecture is adopted for homeedge and more features are separated to create independent module. For example, logmgr is used in every feature almost, copying it in every feature will lead to duplication in all features.

@MoonkiHong
Copy link
Contributor

@tdrozdovsky Looking forward to getting your opinion as well.

@tdrozdovsky
Copy link
Contributor

tdrozdovsky commented Feb 17, 2022

Hello, To make datastorage independent module, I have started the work of cloning the repo of edge orchestration and removing the unnecessary code in terms of datastorage. But as we know few packages like logmgr, networkhelper, dbhelper are being used by storage manager. So as per Golang architecture restrictions, packages mentioned inside the internal package cannot be imported in the independent module. So for the same I have come up with two solutions,

  1. Copy the folders required by datastorage under the lfedge/edge-home-datastorage package i.e. local to datastorage
  2. Move the packages required by the datastorage out of internal folder and create a new folder pkg in edge-home-orchestration and import these packages in datastorage module as
    import https://github.com/lf-edge/edge-home-orchestration-go/pkg/logmgr and so on

Personally I would prefer the second option of moving the packages needed by another module to pkg folder to make it visible for other projects also for import

I think new datastorage container can not access to edge-orchestration db and it would be great if new datastorage is independent from edge-orchestration in terms of source code. Regarding logmgr, it would be better to use edgex logger since we have an issue. I think we can think about as belows.

1. make a rest api for device ip instead of using networkhelper.

2. new datastorage reads device id from the file instead of using dbhelper.

3. implement or copy the source code for others.

I support the option proposed by the @t25kim

@MoonkiHong
Copy link
Contributor

@NiranjanPatil25 @suresh-lc PTAL. Would love to get your opinions as well. 🙏

@nitu-s-gupta
Copy link
Contributor

I started writing the API for getting the deviceIP for Datastorage as independent module
Two scenarios I have analysed,

  1. Home Edge and DataStorage on Same Machine: In this scenario to call API also DataStorage needs to know the IP of HomeEdge, and if it knows the API of Home Edge no need to call API
  2. Home Edge and DataStorage on separate Machine: In this Scenario DataStorage needs to know the IP of Home Edge plus How will Home Edge determine the IP of any unkonwn machine on which its not running. So again API doesnt serve the purpose

So, I would propose since we are copying the logmgr and other functions why not copy this one funciton of networkhelper also in DataStorage locally instead of an API
Please Suggest so that I can proceed,

@tiokim
Copy link
Contributor Author

tiokim commented Feb 21, 2022

  1. Home Edge and DataStorage on separate Machine: In this Scenario DataStorage needs to know the IP of Home Edge plus How will Home Edge determine the IP of any unkonwn machine on which its not running. So again API doesnt serve the purpose

I didn't think about this scenario since I always think Edge Orchestration and DataStorage run on the same machine.
I think Edge Orchestration is mandatory and DataStorage is optional in terms of Home Edge machine.

So, I would propose since we are copying the logmgr

Please see the issue and do not use logmgr for the DataStorage container.

and other functions why not copy this one funciton of networkhelper also in DataStorage locally instead of an API Please Suggest so that I can proceed,

What functions of networkhelper do you need?

@nitu-s-gupta
Copy link
Contributor

  1. Home Edge and DataStorage on separate Machine: In this Scenario DataStorage needs to know the IP of Home Edge plus How will Home Edge determine the IP of any unkonwn machine on which its not running. So again API doesnt serve the purpose

I didn't think about this scenario since I always think Edge Orchestration and DataStorage run on the same machine. I think Edge Orchestration is mandatory and DataStorage is optional in terms of Home Edge machine.

So, I would propose since we are copying the logmgr

Please see the issue and do not use logmgr for the DataStorage container.

and other functions why not copy this one function of network-helper also in Data Storage locally instead of an API Please Suggest so that I can proceed,

What functions of network-helper do you need?

GetOutboundIP() to know the IP of host machine. Yes, I wont use logrmgr but use Edgex foundry logger mechanism. But my point was making API wouldnt help so one function i think we can copy in the datastorage

@tiokim
Copy link
Contributor Author

tiokim commented Feb 22, 2022

  1. Home Edge and DataStorage on separate Machine: In this Scenario DataStorage needs to know the IP of Home Edge plus How will Home Edge determine the IP of any unkonwn machine on which its not running. So again API doesnt serve the purpose

I didn't think about this scenario since I always think Edge Orchestration and DataStorage run on the same machine. I think Edge Orchestration is mandatory and DataStorage is optional in terms of Home Edge machine.

So, I would propose since we are copying the logmgr

Please see the issue and do not use logmgr for the DataStorage container.

and other functions why not copy this one function of network-helper also in Data Storage locally instead of an API Please Suggest so that I can proceed,

What functions of network-helper do you need?

GetOutboundIP() to know the IP of host machine. Yes, I wont use logrmgr but use Edgex foundry logger mechanism. But my point was making API wouldnt help so one function i think we can copy in the datastorage

I would like to have sufficient consideration before putting source code under the pkg folder because of the advice from goland-standard.
It seems good to move networkhelper and dependent code(such as errors, errormsg and logmgr) under pkg. but logmgr would be modified a little.

@nitu-s-gupta
Copy link
Contributor

  1. Home Edge and DataStorage on separate Machine: In this Scenario DataStorage needs to know the IP of Home Edge plus How will Home Edge determine the IP of any unkonwn machine on which its not running. So again API doesnt serve the purpose

I didn't think about this scenario since I always think Edge Orchestration and DataStorage run on the same machine. I think Edge Orchestration is mandatory and DataStorage is optional in terms of Home Edge machine.

So, I would propose since we are copying the logmgr

Please see the issue and do not use logmgr for the DataStorage container.

and other functions why not copy this one function of network-helper also in Data Storage locally instead of an API Please Suggest so that I can proceed,

What functions of network-helper do you need?

GetOutboundIP() to know the IP of host machine. Yes, I wont use logrmgr but use Edgex foundry logger mechanism. But my point was making API wouldnt help so one function i think we can copy in the datastorage

I would like to have sufficient consideration before putting source code under the pkg folder because of the advice from goland-standard. It seems good to move networkhelper and dependent code(such as errors, errormsg and logmgr) under pkg. but logmgr would be modified a little.

logmgr as already mentioned will be using edgex loging mechanism and rest all mentioned uch as errors, errormsg and networkhelper can be moved to pkg folder. I hope this makes sense and I can proceed in this direction?

@tiokim
Copy link
Contributor Author

tiokim commented Feb 22, 2022

logmgr as already mentioned will be using edgex loging mechanism and rest all mentioned uch as errors, errormsg and networkhelper can be moved to pkg folder. I hope this makes sense and I can proceed in this direction?

Sure! I will modify logmgr later after your contribution.

@nitu-s-gupta
Copy link
Contributor

Thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Any tasks and issues w.r.t. the code refactoring v2 v2.0.0
Projects
None yet
Development

No branches or pull requests

6 participants