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.

Sign up for GitHub

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

Jump to bottom

Opinionated API Changes #179

Closed
richardcase opened this issue Oct 28, 2021 · 0 comments · Fixed by #271
Closed

Opinionated API Changes #179

richardcase opened this issue Oct 28, 2021 · 0 comments · Fixed by #271
Assignees
@jmickey
Labels
adr-required Indicates that the issue or PR contains a decision that needs to be documented in a ADR area/api Indicates an issue or PR relates to the APIs kind/feature New feature or request priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now.

Comments

@richardcase
Copy link
Member

Describe the solution you'd like:
Currently the API is generic. We should consider being more opinionated towards our use case. For example, should the user have to specify a network interface for metadata requests? It may be better that if we see metadata in the request then we automatically creatre a network interface for MMDS requests

Why do you want this feature:

Anything else you would like to add:
[Miscellaneous information that will assist in solving the issue.]
@richardcase richardcase added kind/feature New feature or request area/api Indicates an issue or PR relates to the APIs labels Oct 28, 2021
@richardcase richardcase changed the title Opinionate API Changes Opinionated API Changes Oct 28, 2021
@richardcase richardcase added adr-required Indicates that the issue or PR contains a decision that needs to be documented in a ADR priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. labels Nov 4, 2021
@jmickey jmickey self-assigned this Nov 10, 2021
@jmickey jmickey mentioned this issue Nov 18, 2021
Merged
4 tasks
yitsushi added a commit that referenced this issue May 19, 2023
@yitsushi
refactor: resolve/remove TODO comments
caffd11 
* #179 not relevant anymore
* Closes #233 becasue closer inspection, those errors can happen,
  but that does not mean the application has serious issues, the next loop can
  be successful, if it stuck in an error loop, it is visible in logs.
* #206, we have an issue to track that, we don't need an extra comment about it.
* Closes #235, we can return with an error, and the caller can handle it.

Additional changes:
* If any of the main processes returns with an error, stop all other processes,
  for example if `serveAPI` fails, we can stop `runControllers` too.

References:
* #179
* #233
* #206
* #235

Signed-off-by: Balazs Nadasdi <balazs@weave.works>
@yitsushi yitsushi mentioned this issue May 19, 2023
Merged
yitsushi added a commit that referenced this issue May 25, 2023
@yitsushi
refactor: resolve/remove TODO comments (#709)
eb239ae 
* #179 not relevant anymore
* Closes #233 becasue closer inspection, those errors can happen,
  but that does not mean the application has serious issues, the next loop can
  be successful, if it stuck in an error loop, it is visible in logs.
* #206, we have an issue to track that, we don't need an extra comment about it.
* Closes #235, we can return with an error, and the caller can handle it.

Additional changes:
* If any of the main processes returns with an error, stop all other processes,
  for example if `serveAPI` fails, we can stop `runControllers` too.

References:
* #179
* #233
* #206
* #235

Signed-off-by: Balazs Nadasdi <balazs@weave.works>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jmickey jmickey

Labels
adr-required Indicates that the issue or PR contains a decision that needs to be documented in a ADR area/api Indicates an issue or PR relates to the APIs kind/feature New feature or request priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

refactor: apply opinionated api defaults based on accepted adr liquidmetal-dev/flintlock
2 participants
@richardcase @jmickey