Skip to content

feat: Add proxy to build script for device-usb-service#271

Merged
presatish merged 1 commit intoedgexfoundry:mainfrom
EdgeX-Camera-Management:usb_proxy_add
Aug 31, 2023
Merged

feat: Add proxy to build script for device-usb-service#271
presatish merged 1 commit intoedgexfoundry:mainfrom
EdgeX-Camera-Management:usb_proxy_add

Conversation

@seanohair22
Copy link
Copy Markdown
Contributor

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?)
    Docs PR for proxy

Testing Instructions

Add proxy to docker (ping me for specific proxy setup instructions)
Test onsite or on VPN
Run make docker, the service should build.

New Dependency Instructions (If applicable)

Signed-off-by: Sean O'Hair <sean.ohair@intel.com>
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

Merging #271 (02f39ed) into main (860b2ed) will not change coverage.
The diff coverage is n/a.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@          Coverage Diff          @@
##            main    #271   +/-   ##
=====================================
  Coverage   4.83%   4.83%           
=====================================
  Files          8       8           
  Lines       1283    1283           
=====================================
  Hits          62      62           
  Misses      1221    1221           

Comment on lines +36 to +38
--build-arg http_proxy=$(HTTP_PROXY) \
--build-arg https_proxy=$(HTTPS_PROXY) \
--build-arg no_proxy=$(NO_PROXY) \
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should these use the lowercase form as well? aka --build-arg http_proxy=$(http_proxy), which i think can just be shortened to --build-arg http_proxy

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I followed what was present in the device-onvif-service, that way the environment variables only need to be set with one format for building both services...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ok, that seems fair, as long as the documentation matches up, which I believe it does when I reviewed that last week.

Copy link
Copy Markdown
Contributor

@presatish presatish left a comment

Choose a reason for hiding this comment

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

The changes look fine but I am not able to test as it looks like vpn certificate for my linux machine has been expired or has some issues because of which I am unable to test.

@presatish presatish merged commit 9c60bf2 into edgexfoundry:main Aug 31, 2023
@presatish presatish deleted the usb_proxy_add branch August 31, 2023 19:35
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.

4 participants