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

build(snap): create the required kuiper connections directory #3785

Closed

Conversation

farshidtz
Copy link
Member

@farshidtz farshidtz commented Oct 27, 2021

Create the kuiper connections directory required by the secretstore, added by #3778.

This was causing a runtime fatal error for the security-secretstore-setup, effectively preventing the installation of edgexfoundry snap.

For the record, IMO, the directory creation should be part of the application logic and not the packaging.

See: #3778 (comment)
Closes: #3786

We may wanna remove this when moving to eKuiper 1.4.0 because the directory already exists there.

Signed-off-by: Farshid Tavakolizadeh [email protected]

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

$ snapcraft
$ sudo snap install ./edgexfoundry_2.0.1-dev.69_amd64.snap --dangerous
edgexfoundry 2.0.1-dev.69 installed

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 A 0 Security Hotspots
Code Smell A 0 Code Smells

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

@lenny-goodell
Copy link
Member

For the record, IMO, the directory creation should be part of the application logic and not the packaging.

@farshidtz , @tonyespy . I agree with this statement. I can submit a fix for the bug that first tests for the existence of the folder and creates it if it doesn't exist.

@farshidtz
Copy link
Member Author

@farshidtz , @tonyespy . I agree with this statement. I can submit a fix for the bug that first tests for the existence of the folder and creates it if it doesn't exist.

That sounds good, os.MkdirAll does just that.

I guess adding an option to completely disable that is also an option: set whenever Kuiper 1.3.x, remote Kuiper, or no Kuiper is used. I didn't check, but maybe unsetting the environment variable already does that?

@lenny-goodell
Copy link
Member

That sounds good, os.MkdirAll does just that.

I guess adding an option to completely disable that is also an option: set whenever Kuiper 1.3.x, remote Kuiper, or no Kuiper is used. I didn't check, but maybe unsetting the environment variable already does that?

I have gone with skipping if the file doesn't already exist. That way if in future release they move away from the Sources file, it will not break.

@farshidtz
Copy link
Member Author

That sounds good, os.MkdirAll does just that.
I guess adding an option to completely disable that is also an option: set whenever Kuiper 1.3.x, remote Kuiper, or no Kuiper is used. I didn't check, but maybe unsetting the environment variable already does that?

I have gone with skipping if the file doesn't already exist. That way if in future release they move away from the Sources file, it will not break.

Awesome. Thanks @lenny-intel

I am closing this PR in favor of #3787.

@farshidtz farshidtz closed this Oct 28, 2021
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.

New eKuiper credentials injection into Connections file cause snaps to fail to install
2 participants