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

Acap runtime proto image #77

Closed
wants to merge 13 commits into from
Closed

Acap runtime proto image #77

wants to merge 13 commits into from

Conversation

Corallo
Copy link
Contributor

@Corallo Corallo commented Jul 12, 2024

Describe your changes

Adding a new dockerfile to ACAP runtime that builds proto APIs for python.

Angelo DelliSanti added 2 commits July 12, 2024 11:02
Change-Id: I4cb8389f457fa33fca8109da70238f09ec5851a1
Change-Id: Ie7e63c3c42ac135a6b807819691b592b77a96ca3
@Corallo Corallo requested review from a team as code owners July 12, 2024 09:14
Change-Id: I02e4bedc5cfee51acf1c1cb0ba7ddb7581dcfa5d
Change-Id: I2c635bd6044806e1357d43b9f4a6acf50de5ea9b
@@ -21,7 +21,7 @@ package keyvaluestore;
// A simple key-value storage service
service KeyValueStore {
// Provides a value for each key request
rpc GetValues (stream Request) returns (stream Response) {}
rpc GetValues (Request) returns (Response) {}
Copy link
Contributor

Choose a reason for hiding this comment

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

This requires the parameter api code and test to be updated as well

Copy link
Contributor

Choose a reason for hiding this comment

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

This change is pushed to main now so you can rebase this PR to pick it up
NB! we do currently have an issue on the unit tests for parameter API on main though, so tests are failing there

Copy link
Contributor

Choose a reason for hiding this comment

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

tests are fixed now

Copy link
Contributor

@madelen-at-work madelen-at-work left a comment

Choose a reason for hiding this comment

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

Dockerfile.proto looks good. But the plan is to publish it to Docker Hub as well, right? If so a workflow for that would be nice

@Corallo
Copy link
Contributor Author

Corallo commented Aug 15, 2024

Dockerfile.proto looks good. But the plan is to publish it to Docker Hub as well, right? If so a workflow for that would be nice

Should it be part of the ci-cd?
https://github.com/AxisCommunications/acap-runtime/blob/main/.github/workflows/ci-cd.yml

@madelen-at-work
Copy link
Contributor

Dockerfile.proto looks good. But the plan is to publish it to Docker Hub as well, right? If so a workflow for that would be nice

Should it be part of the ci-cd? https://github.com/AxisCommunications/acap-runtime/blob/main/.github/workflows/ci-cd.yml

Might be cleaner to have a separate file

@Corallo
Copy link
Contributor Author

Corallo commented Aug 19, 2024

Dockerfile.proto looks good. But the plan is to publish it to Docker Hub as well, right? If so a workflow for that would be nice

Should it be part of the ci-cd? https://github.com/AxisCommunications/acap-runtime/blob/main/.github/workflows/ci-cd.yml

Might be cleaner to have a separate file

@madelen-at-work I was thinking that dockerhub has a functionality to automatically rebuild for instance on tag.
This is what we do at the moment for the computer-vision-sdk. Do you think there is some advantage in making a github workflow instead of setting up dockerhub to do the build for that dockerfile?

@madelen-at-work
Copy link
Contributor

Dockerfile.proto looks good. But the plan is to publish it to Docker Hub as well, right? If so a workflow for that would be nice

Should it be part of the ci-cd? https://github.com/AxisCommunications/acap-runtime/blob/main/.github/workflows/ci-cd.yml

Might be cleaner to have a separate file

@madelen-at-work I was thinking that dockerhub has a functionality to automatically rebuild for instance on tag. This is what we do at the moment for the computer-vision-sdk. Do you think there is some advantage in making a github workflow instead of setting up dockerhub to do the build for that dockerfile?

When we open source:d this project we deliberately moved building from Docker Hub to GitHub to be able to keep everything contained in this repo. I would prefer not setting up a new flow in Docker Hub.

Dockerfile.proto Show resolved Hide resolved
Dockerfile.proto Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Dockerfile.proto Outdated Show resolved Hide resolved
Dockerfile.proto Outdated Show resolved Hide resolved
Angelo Delli Santi and others added 3 commits August 27, 2024 12:44
Co-authored-by: Patrik Åkesson <[email protected]>
Co-authored-by: Patrik Åkesson <[email protected]>
Change-Id: I353ec4f90058322b87f8a24ebfdef9c906640377
Dockerfile.proto Show resolved Hide resolved
Dockerfile.proto Outdated Show resolved Hide resolved
Angelo Delli Santi and others added 4 commits August 29, 2024 10:04
Co-authored-by: Patrik Åkesson <[email protected]>
Change-Id: I4dce46b382d18d718c6c125f24f245caf887cb7e
wip
Change-Id: Ic4e6c7047872d93ea8f0087568d98eba8e171ee4
@johan-olsson-work johan-olsson-work mentioned this pull request Aug 30, 2024
6 tasks
@johan-olsson-work
Copy link
Contributor

Before merging this PR make sure the following remaining parts from closed PR #86 is added:

  • make sure platform is correctly selected in the workflow to avoid this warning

@johan-olsson-work
Copy link
Contributor

Squashed into one commit and rebased towards main in #96 instead.

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

Successfully merging this pull request may close these issues.

5 participants