Skip to content

Conversation

@hweihwang
Copy link
Contributor

Steps to run:

occ app_api:app:register whiteboard_websocket --info-xml=./apps/whiteboard/websocket_server/appinfo/info.xml

@hweihwang hweihwang marked this pull request as ready for review October 15, 2024 11:58
@hweihwang hweihwang marked this pull request as draft October 15, 2024 11:58
Copy link
Member

@juliusknorr juliusknorr left a comment

Choose a reason for hiding this comment

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

Some early comments and questions, but looks promising :)

@hweihwang hweihwang requested a review from juliusknorr October 21, 2024 12:07
@hweihwang hweihwang force-pushed the whiteboard-exapp branch 5 times, most recently from f7f51f9 to 67c5c40 Compare October 22, 2024 09:54
@hweihwang hweihwang marked this pull request as ready for review October 22, 2024 09:54
@hweihwang hweihwang linked an issue Oct 24, 2024 that may be closed by this pull request
3 tasks
hweihwang and others added 2 commits February 26, 2025 08:11
Signed-off-by: Hoang Pham <hoangmaths96@gmail.com>
PORT: process.env.PORT || DEFAULT_PORT,
IS_DEV: Utils.parseBooleanFromEnv(process.env.IS_DEV),

SETUP_TYPE: process.env.SETUP_TYPE || DEFAULT_SETUP_TYPE,
Copy link
Member

Choose a reason for hiding this comment

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

Not a blocker: I'm wondering if we could detect this from any of the env variables that app_api injects so we don't need to set this explicitly on the docker image. I would prefer if we could have a single multi purpose docker image.

Copy link
Member

Choose a reason for hiding this comment

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

I did a similar thing with the APP_PORT below, as the previously hardcoded one 23000 did not work if you had another ex app installed before.

@@ -0,0 +1,24 @@
# SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this extra file?

@juliusknorr
Copy link
Member

Tested and works well 👍

Before merging we should decide if we can wrap this into the existing docker image or if we need to build a separate one.

  • Have just one docker file or add a separate github action to build the second one

After merge

  • Make sure that the version of both regular app and ex app is in sync when building
  • Do a first release and document the steps needed
  • Look into how we can automate that

@hweihwang
Copy link
Contributor Author

Closed. Move the future developments to #467 cc @juliusknorr @grnd-alt

@hweihwang hweihwang closed this Jun 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Investigate ext app for node backend

4 participants