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

Flexibility refactor #9

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open

Flexibility refactor #9

wants to merge 23 commits into from

Conversation

ajbalogh
Copy link
Collaborator

@ajbalogh ajbalogh commented Nov 12, 2024

The current version of infra.proto is not flexible enough to accommodate better device reuse, more complex topologies and external third party data. At this time there is also no specific rpc for model validation. In addition there is code that builds concrete infrastructure objects that is application specific and should not be part of this repository.

This PR will address the following:

  • removal of application specific samples
  • binding proto that allows application specific data to be bound to various infrastructure paths
  • service proto that initially provides a validation rpc
  • validation rpc implemented as a class intended for local use only (no grpc server/client needed)
  • unit tests focused on model validation
  • additional comments in proto files

@ajbalogh ajbalogh added the enhancement New feature or request label Nov 12, 2024
@ajbalogh ajbalogh self-assigned this Nov 12, 2024
@ajbalogh ajbalogh linked an issue Nov 12, 2024 that may be closed by this pull request
@thomas-am
Copy link
Collaborator

It would be nice to show the intended use, and give a couple examples what to use bindings for...
Either in readme or as a test case.

@ajbalogh
Copy link
Collaborator Author

It would be nice to show the intended use, and give a couple examples what to use bindings for... Either in readme or as a test case.

Added a few examples at the top of the bind.proto file

protos/service.proto Outdated Show resolved Hide resolved
src/service.py Outdated Show resolved Hide resolved
@ajbalogh ajbalogh marked this pull request as ready for review November 14, 2024 22:32
@ajbalogh
Copy link
Collaborator Author

ajbalogh commented Dec 3, 2024

Removed the linktype as it adds overhead and opens the link message to interpretation. The bandwidth message in the link is the most important piece and is required. A description field has been added instead so that additional details describing the link can be provided. To categorize the link the bind.proto should be used. @harsh-sikhwal

@ajbalogh
Copy link
Collaborator Author

ajbalogh commented Dec 4, 2024

Revised the bind.proto to allow a user to specify multiple bind targets to one user defined data message, prior to this change you had to repeat the user defined message if it was to be applied to multiple disparate instances.

@Ricefrog
Copy link
Collaborator

Ricefrog commented Dec 4, 2024

In general, I approve of the change. I think this technically covers every way that a binding might be attached to a subset of the entire infrastructure, with a potential drawback being targets lists that are very large.

@ajbalogh
Copy link
Collaborator Author

ajbalogh commented Dec 5, 2024

In general, I approve of the change. I think this technically covers every way that a binding might be attached to a subset of the entire infrastructure, with a potential drawback being targets lists that are very large.

re: target lists can get large - this buys time to evaluate alternate scaling techniques for indexing and avoid api churn

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Current infra.proto is too rigid
4 participants