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.

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

Clientlog Service #7217

Merged
merged 6 commits into from Sep 7, 2023
Merged

Clientlog Service #7217

merged 6 commits into from Sep 7, 2023

Conversation

kobergj
Copy link
Collaborator

@kobergj kobergj commented Sep 5, 2023

Introduces the clientlog service that send machine readable notifications to clients

@kobergj
Copy link
Collaborator Author

kobergj commented Sep 5, 2023

  • Provide a README.md for that service in the root folder of that service.
    • Use CamelCase for section headers.
  • For images and example files used in README.md:
    • Create a folder named md-sources on the same level where README.md is located. Put all the images and example files referenced by README.md into this folder.
    • Use absolute references like https://raw.githubusercontent.com/owncloud/ocis/master/services/<service-name>/md-sources/file to make the content accessible for both README.md and owncloud.dev
      bad <img src="https://github.com/owncloud/ocis/blob/master/services/graph/images/mermaid-graph.svg" width="500" />
      good <img src="https://raw.githubusercontent.com/owncloud/ocis/master/services/graph/images/mermaid-graph.svg" width="500" />
  • If new CLI command are introduced, that command must be described in readme.md.
  • If new global envvar is introduced, the name must start with OCIS_.
  • Add the service to the makefile in the ocis repo root.
  • Make the service startable for binary and individual startup:
    • For single binary add service to ocis/pkg/runtime
    • For individual startup add service to ocis/pkg/commands
  • Add the service to .drone.star to enable CI.
  • Inform doc team in an early stage to review the readme AND the environment variables created.
    • The description must reflect the behaviour AND usually has a positive code quality impact.
  • Create proper description strings for envvars - see other services for examples, especially when it comes to multiple values. This must include:
    • base description, set of available values, description of each value.
  • When suggestable commits are created for text changes and you agree, collect them to a batch and commit them. Do not forget to rebase locally to avoid overwriting the changes made.
  • If new envvars are introduced which serve the same purpose but in multiple services, an additional envvar must be added at the beginning of the list starting with OCIS_ (global envvar).
  • Ensure that a service has a debug port
  • If the new service introduces a new port:
  • Make sure to have a function FullDefaultConfig() in pkg/config/defaults/defaultconfig.go of your service. It is needed to create the documentation.

@kobergj kobergj force-pushed the ClientlogService branch 5 times, most recently from 0b3dc4d to 1ba5acb Compare September 5, 2023 09:54
Comment on lines +20 to +24
// ClientNotification is the event the clientlog service is sending to the client
type ClientNotification struct {
Type string
ItemID string
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@felix-schwarz I was told you already thought about sses and how they should look like. Unfortunately nobody could tell me where you have written them down. This is what we have so far for client notifications. It's very basic but we can iterate over it.

Any suggestions/remarks/comments from your side?

kobergj and others added 5 commits September 7, 2023 10:56
Signed-off-by: jkoberg <jkoberg@owncloud.com>
Signed-off-by: jkoberg <jkoberg@owncloud.com>
Co-authored-by: Martin <github@diemattels.at>
Signed-off-by: jkoberg <jkoberg@owncloud.com>
Signed-off-by: jkoberg <jkoberg@owncloud.com>
Copy link
Contributor

@fschade fschade left a comment

Choose a reason for hiding this comment

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

nothing crucial, up to you

testing... not sure, not much business logic involved

services/clientlog/pkg/command/server.go Outdated Show resolved Hide resolved
@@ -0,0 +1,3 @@
package service_test

// TODO: TEST!
Copy link
Contributor

Choose a reason for hiding this comment

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

... ;)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not much business logic. Mocking services would be a nightmare. Removed the _test.go files

Signed-off-by: jkoberg <jkoberg@owncloud.com>
@sonarcloud
Copy link

sonarcloud bot commented Sep 7, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

8.8% 8.8% Coverage
15.4% 15.4% Duplication

@kobergj kobergj marked this pull request as ready for review September 7, 2023 13:25
@kobergj kobergj merged commit 1fe1805 into owncloud:master Sep 7, 2023
2 checks passed
@kobergj kobergj deleted the ClientlogService branch September 7, 2023 13:26
ownclouders pushed a commit that referenced this pull request Sep 7, 2023
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.

None yet

4 participants