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

refactor(snap): Remove explicit config provider addresses #4396

Merged
merged 1 commit into from
Mar 1, 2023

Conversation

farshidtz
Copy link
Member

@farshidtz farshidtz commented Feb 28, 2023

Given the current flags, the services (core-metadata, core-command, support-scheduler, support-notifications) don't get registered in Consul and don't produce any errors either.

There appear to be a bug either in the command processing of snapcraft or the EdgeX services. It appear that the --registry flag isn't processed at all. This PR is set as a refactoring type because the removed flag values are the defaults and theoretically equivalent.

Another working solution was to change the order from --configProvider consul.http://localhost:8500/ --registry to --registry --configProvider consul.http://localhost:8500/.

If your build fails due to your commit message not passing the build checks, please review the guidelines here: https://github.com/edgexfoundry/edgex-go/blob/main/.github/Contributing.md

PR Checklist

Please check if your PR fulfills the following requirements:

  • I am not introducing a breaking change (if you are, flag in conventional commit message with BREAKING CHANGE: describing the break)
  • I am not introducing a new dependency (add notes below if you are)
  • I have added unit tests for the new feature or bug fix (if not, why?)
  • I have fully tested (add details below) this the new feature or bug fix (if not, why?)
  • I have opened a PR for the related docs change (if not, why?)

Testing Instructions

Before:

$ snap logs -f edgexfoundry
...
2023-02-28T17:10:09+01:00 edgexfoundry.consul[210169]: 2023-02-28T17:10:09.531+0100 [INFO]  agent: Synced node info
2023-02-28T17:10:17+01:00 edgexfoundry.consul[210169]: 2023-02-28T17:10:17.581+0100 [INFO]  agent: Synced check: check=core-data
^C

After:

$ snap logs -f edgexfoundry
...
2023-02-28T17:13:34+01:00 edgexfoundry.consul[212989]: 2023-02-28T17:13:34.017+0100 [INFO]  agent: Synced check: check=support-scheduler
2023-02-28T17:13:35+01:00 edgexfoundry.consul[212989]: 2023-02-28T17:13:35.079+0100 [INFO]  agent: Synced check: check=core-data
2023-02-28T17:13:35+01:00 edgexfoundry.consul[212989]: 2023-02-28T17:13:35.698+0100 [INFO]  agent: Synced check: check=core-command
2023-02-28T17:13:38+01:00 edgexfoundry.consul[212989]: 2023-02-28T17:13:38.687+0100 [INFO]  agent: Synced check: check=support-notifications
2023-02-28T17:13:41+01:00 edgexfoundry.consul[212989]: 2023-02-28T17:13:41.448+0100 [INFO]  agent: Synced check: check=core-metadata
^C

New Dependency Instructions (If applicable)

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

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

No Coverage information No Coverage information
2.0% 2.0% Duplication

@farshidtz farshidtz marked this pull request as ready for review February 28, 2023 16:14
Copy link
Member

@lenny-goodell lenny-goodell left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@MonicaisHer MonicaisHer left a comment

Choose a reason for hiding this comment

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

@farshidtz Thank you, looks good!

@farshidtz farshidtz merged commit c62a5d8 into edgexfoundry:main Mar 1, 2023
@farshidtz farshidtz deleted the rm-config-provider-path branch March 1, 2023 09:11
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