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

Add stream management HTTP api #180

Merged
merged 29 commits into from
Dec 14, 2023
Merged

Add stream management HTTP api #180

merged 29 commits into from
Dec 14, 2023

Conversation

PawelPeczek-Roboflow
Copy link
Collaborator

@PawelPeczek-Roboflow PawelPeczek-Roboflow commented Dec 8, 2023 •
edited
Loading

Description

Please see documentation that was created!

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

How has this change been tested, please provide a testcase or example of how you tested the change?

  • suite of unit tests
  • end 2 end on CPU machine
  • planned: e2e on Jetson and GPU - results will be delivered soon

Any specific deployment considerations

Docs

  • Docs updated? What were the changes:

github-advanced-security[bot]
github-advanced-security bot found potential problems Dec 11, 2023
View reviewed changes
inference/enterprise/stream_management/api/app.py Dismissed Show dismissed Hide dismissed
inference/enterprise/stream_management/api/app.py Dismissed Show dismissed Hide dismissed
inference/enterprise/stream_management/api/app.py Dismissed Show dismissed Hide dismissed
inference/enterprise/stream_management/api/app.py Dismissed Show dismissed Hide dismissed
inference/enterprise/stream_management/api/app.py Dismissed Show dismissed Hide dismissed
@PawelPeczek-Roboflow PawelPeczek-Roboflow marked this pull request as ready for review December 12, 2023 15:29
@PawelPeczek-Roboflow PawelPeczek-Roboflow added the documentation Improvements or additions to documentation label Dec 12, 2023
@PawelPeczek-Roboflow
Copy link
Collaborator Author

Verified that it works under Linux/GPU

@PawelPeczek-Roboflow
Copy link
Collaborator Author

Confirmed that it works e2e at jetson device

paulguerrie
paulguerrie approved these changes Dec 14, 2023
View reviewed changes
Copy link
Contributor

@paulguerrie paulguerrie left a comment

Choose a reason for hiding this comment

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

Looks good to me! Left some comments, but no show stoppers.

.github/workflows/docker.stream_management_api.yml Outdated Show resolved Hide resolved
.github/workflows/docker.stream_manager.cpu.yml Outdated Show resolved Hide resolved
.github/workflows/docker.stream_manager.gpu.yml Outdated Show resolved Hide resolved
.github/workflows/docker.stream_manager.jetson.5.1.1.yml Outdated Show resolved Hide resolved
docker/dockerfiles/stream-management-api.compose-cpu.yaml Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Just had a thought: Is it possible to host both the stream manager and the stream manager API in the same docker container and just run them each as separate processes?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we could although I just separated based on the common good practices: https://docs.docker.com/config/containers/multi-service_container/

@PawelPeczek-Roboflow PawelPeczek-Roboflow merged commit 3642929 into main Dec 14, 2023
5 checks passed
@PawelPeczek-Roboflow PawelPeczek-Roboflow deleted the feature/add_stream_management_api branch December 14, 2023 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@paulguerrie paulguerrie paulguerrie approved these changes

@probicheaux probicheaux Awaiting requested review from probicheaux probicheaux is a code owner

@capjamesg capjamesg Awaiting requested review from capjamesg

Assignees
No one assigned
Labels
documentation Improvements or additions to documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@PawelPeczek-Roboflow @paulguerrie