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

Tweak go-micro behavior #840

Merged
merged 13 commits into from
Nov 12, 2020
Merged

Tweak go-micro behavior #840

merged 13 commits into from
Nov 12, 2020

Conversation

refs
Copy link
Member

@refs refs commented Nov 11, 2020

There are a few building blocks that we were relying on default behavior, such as micro.Registry and the go-micro client. In order for oCIS to work in any environment and not relying in black magic configuration or running daemons we need to be able to:

  • Provide with a configurable go-micro registry.
  • Use our own go-micro client adjusted to our own needs (i.e: custom timeout, custom dial timeout, custom transport...)

This PR is relying on 2 env variables from Micro: MICRO_REGISTRY and MICRO_REGISTRY_ADDRESS. The latter does not make sense to provide if the registry is not etcd.

The current implementation only accounts for mdns and etcd registries, defaulting to mdns when not explicitly defined to use etcd.

The new custom client raises the timeout from 5 to 10 seconds.

Try It!

  1. checkout this branch
  2. start etcd
  3. run ocis: on owncloud/ocis/ocis -> > MICRO_REGISTRY=etcd go run cmd/ocis/main.go server
  4. open your browser and do the things you usually do, like login and uploading funny gifs

giphy

TODO:

  • get to the bottom of com.owncloud.api.storage using mdns

Disclaimer:

When using etcd deregistering services might take a while.

@update-docs
Copy link

update-docs bot commented Nov 11, 2020

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@IljaN IljaN self-requested a review November 11, 2020 14:52
Copy link
Member

@kulmann kulmann left a comment

Choose a reason for hiding this comment

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

🚀

@refs refs self-assigned this Nov 11, 2020
@phil-davis
Copy link
Contributor

I am tempted to press merge, but there is no changelog.
Should this have a changelog?

@refs
Copy link
Member Author

refs commented Nov 12, 2020

I am tempted to press merge, but there is no changelog.
Should this have a changelog?

yes, please do not merge yet, still got changes. I should get in the habit of drafting PR's :)

@butonic butonic mentioned this pull request Nov 12, 2020
@kulmann kulmann requested review from IljaN and kulmann November 12, 2020 10:34
@kulmann
Copy link
Member

kulmann commented Nov 12, 2020

I dismissed the reviews/approvals since you added commits. Ready for review? :-)

@refs
Copy link
Member Author

refs commented Nov 12, 2020

I dismissed the reviews/approvals since you added commits. Ready for review? :-)

@kulmann sure!

@dragotin
Copy link
Contributor

Works on openSUSE Tumbleweed with etcd started by systemd from the (manually fixed) package.

Seeing this log line in the sys logs tells me that etcd is really used:

etcd[13905]: serving insecure client requests on 127.0.0.1:2379, this is strongly discouraged!

Uploading funny gif works, trying to view it results in an rather unfunny experience ;-)

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@refs refs merged commit f90d0d6 into master Nov 12, 2020
@delete-merged-branch delete-merged-branch bot deleted the ocis-1018 branch November 12, 2020 12:59
ownclouders pushed a commit that referenced this pull request Nov 12, 2020
Merge: 5a49241 641fd27
Author: Alex Unger <[email protected]>
Date:   Thu Nov 12 13:59:12 2020 +0100

    Merge pull request #840 from owncloud/ocis-1018

    Tweak go-micro behavior
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.

5 participants