Skip to content

Feature: client push - #551

Open
AndreKoepke wants to merge 4 commits into
nextcloud:mainfrom
AndreKoepke:feature/client_push
Open

Feature: client push#551
AndreKoepke wants to merge 4 commits into
nextcloud:mainfrom
AndreKoepke:feature/client_push

Conversation

@AndreKoepke

@AndreKoepke AndreKoepke commented Apr 2, 2024

Copy link
Copy Markdown

Pull Request

Description of the change

Add a container for client_push app. It allows websocket-connections for updates instead of polling. See notify_push

Benefits

  • notify_push is usable

Possible drawbacks

  • Nextcloud-Chart must be started without notify_push, so it can be installed.
  • Then Next-Chart can be started with notify_push
  • After that, notify_push must be manually activated by running sudo -u www-data ./occ notify_push:setup https://NEXTCLOUD_HOST/push

Applicable issues

Additional information

Checklist

@AndreKoepke

Copy link
Copy Markdown
Author

Can we install apps with commands? If so, it can be added as default.

image: {{ include "nextcloud.image" . }}
imagePullPolicy: {{ .Values.image.pullPolicy }}
command:
- custom_apps/notify_push/bin/x86_64/notify_push

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Hard-coded x86_64

@AndreKoepke
AndreKoepke force-pushed the feature/client_push branch from c4ce530 to d57a66d Compare April 2, 2024 12:49
Signed-off-by: Andre Köpke <github@akop.online>
@AndreKoepke
AndreKoepke force-pushed the feature/client_push branch from d57a66d to 9d79183 Compare April 2, 2024 12:54
@Leptopoda

Copy link
Copy Markdown
Member

Can we install apps with commands? If so, it can be added as default.

there is occ app:install notify_push
https://docs.nextcloud.com/server/stable/admin_manual/configuration_server/occ_command.html#apps-commands-label

@wrenix

wrenix commented Jun 9, 2024

Copy link
Copy Markdown
Collaborator

I really like the idea to run notify_push.

But please in a different way:

  • extra deployment (like for metrics)
  • extra service and ingress with Additional path
  • improve service monitor to track that metrics also

The reason:

  • On this way, we could scale nextcloud indepenten fron push client
  • And do not maintain nginx or Apache code, that could handle ingress-controller.

Another reason (i need to verify / hope that it exists), we could use a other image for the client push (without any distro inside thanks Rust) and track a more beautiful/helm-like way the version. (That image exists but is not official yet: nextcloud/notify_push#106 (comment)) PS: it will also solve the CPU architecture problem (the notify_push image should just build in all targets under the same container label).


i start to write, what i mean under: #581
if you like to test it or use that as first state to improve your PR take it

@AndreKoepke

AndreKoepke commented Jun 12, 2024

Copy link
Copy Markdown
Author

i start to write, what i mean under: #581 if you like to test it or use that as first state to improve your PR take it

I like your changes. I'm running my variant since I opened the PR, but I will switch to your variant in the next days.
Should we (or I) close my PR?

@wrenix

wrenix commented Jun 14, 2024

Copy link
Copy Markdown
Collaborator

Till there is a solution merged i would keep it open in your place.

Comment thread charts/nextcloud/templates/deployment.yaml
@jessebot jessebot changed the title Feature/client push Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants