Skip to content

Fishjam foundry example#59

Draft
roznawsk wants to merge 4 commits intomainfrom
fishjam-foundry-example
Draft

Fishjam foundry example#59
roznawsk wants to merge 4 commits intomainfrom
fishjam-foundry-example

Conversation

@roznawsk
Copy link
Member

@roznawsk roznawsk commented Mar 4, 2026

No description provided.

Copy link
Member

Choose a reason for hiding this comment

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

suggestion: Generating a client from the openapi instead would be preferred and more educational imo

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried to do that with composition api, but failed. The api is generated for Fishjam though

Copy link
Member

Choose a reason for hiding this comment

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

praise: I'm pretty sure we can just use useLivestreamViewer from @fishjam-cloud/react-client with token and url

@roznawsk roznawsk force-pushed the fishjam-foundry-example branch from ce58d4b to 508b183 Compare March 4, 2026 16:40
@roznawsk roznawsk force-pushed the fishjam-foundry-example branch from 508b183 to 9786ca4 Compare March 4, 2026 16:46
@@ -0,0 +1,11 @@
module conference-to-stream

go 1.24.5
Copy link
Member

Choose a reason for hiding this comment

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

suggestion: probably should be the same as Dockerfile


// Start WS notifier for this room
fishjamBaseURL := h.fishjamClient.BaseURL()
_, err = fishjam.NewNotifier(fishjamBaseURL, h.managementToken, fishjam.NotifierCallbacks{
Copy link
Member

Choose a reason for hiding this comment

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

issue: memleak

PeerWebsocketURL string `json:"peerWebsocketUrl"`
}

func (h *Handler) CreateRoom(w http.ResponseWriter, r *http.Request) {
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: there is still very much a race condition here

@@ -0,0 +1,15 @@
FROM node:22-alpine AS builder
WORKDIR /app
COPY package.json ./
Copy link
Member

Choose a reason for hiding this comment

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

issue: should use lockfile

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.

3 participants