Skip to content

Feat/qwen speech summarization#255

Open
eric-mccann-pro wants to merge 9 commits intodevelopfrom
feat/qwen-speech-summarization
Open

Feat/qwen speech summarization#255
eric-mccann-pro wants to merge 9 commits intodevelopfrom
feat/qwen-speech-summarization

Conversation

@eric-mccann-pro
Copy link

@eric-mccann-pro eric-mccann-pro commented Jan 5, 2026

@eric-mccann-pro eric-mccann-pro self-assigned this Jan 5, 2026
@eric-mccann-pro eric-mccann-pro marked this pull request as draft January 5, 2026 19:16
@eric-mccann-pro eric-mccann-pro marked this pull request as ready for review January 5, 2026 19:23
Copy link
Member

@jrobble jrobble left a comment

Choose a reason for hiding this comment

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

@jrobble made 2 comments.
Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on @eric-mccann-pro).


docker-compose.components.yml line 228 at r1 (raw file):

    build: ${OPENMPF_PROJECTS_PATH}/openmpf-components/python/QwenSpeechSummarization

  # TODO: Rename this to qwen-speech-summarization-server

Do what's mentioned in this TODO.


docker-compose.components.yml line 244 at r1 (raw file):

    tty: true
    ipc: host
    # volumes:

Can we remove these fields?

    tty: true
    ipc: host
    # volumes:

Copy link
Member

@jrobble jrobble left a comment

Choose a reason for hiding this comment

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

@jrobble reviewed 1 file and all commit messages, and resolved 2 discussions.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @eric-mccann-pro).

Copy link

@tstrass tstrass left a comment

Choose a reason for hiding this comment

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

@tstrass reviewed all commit messages and made 2 comments.
Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on eric-mccann-pro and jrobble).


docker-compose.components.yml line 226 at r3 (raw file):

    <<: *component-base
    image: ${REGISTRY}openmpf_llm_speech_summarization:${TAG}
    build: ${OPENMPF_PROJECTS_PATH}/openmpf-components/python/LlmSpeechSummarization

Would we want this to depends_on the vllm server?


docker-compose.components.yml line 240 at r3 (raw file):

      #       - driver: nvidia
      #         device_ids: ['0']
      #         capabilities: [gpu]

Why is resources commented out?

Copy link
Author

@eric-mccann-pro eric-mccann-pro left a comment

Choose a reason for hiding this comment

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

@eric-mccann-pro made 2 comments.
Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on jrobble and tstrass).


docker-compose.components.yml line 226 at r3 (raw file):

Previously, tstrass (Tom Strassner) wrote…

Would we want this to depends_on the vllm server?

Nope! It could be pointed at an external VLLM server and vllm starts up so slow that the latebound healthcheck keeps things cleaner IMO


docker-compose.components.yml line 240 at r3 (raw file):

Previously, tstrass (Tom Strassner) wrote…

Why is resources commented out?

I think that was a robble decision. I think it's to have the file's contents be tailorable in the step where a user of the repo uses "docker compose config" to expand the templates and then choose which components they actually want to use

Copy link

@tstrass tstrass left a comment

Choose a reason for hiding this comment

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

@tstrass resolved 2 discussions.
Reviewable status: 0 of 1 files reviewed, all discussions resolved (waiting on jrobble).

Copy link

@tstrass tstrass left a comment

Choose a reason for hiding this comment

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

@tstrass reviewed 1 file.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on eric-mccann-pro).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

3 participants