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

Proxy install #711

Merged
merged 10 commits into from
Aug 28, 2023
Merged

Proxy install #711

merged 10 commits into from
Aug 28, 2023

Conversation

teclator
Copy link
Contributor

Problem

In #696 we added support for configuring the proxy when booting the installation system but the configuration is lost after the installation.

Solution

The proxy configuration will be copied to the target system when a proxy is used as well as the microos-tools package will be installed.

Testing

  • Added a new unit test
  • Tested manually

@coveralls
Copy link

coveralls commented Aug 22, 2023

Coverage Status

coverage: 72.182% (+0.08%) from 72.1% when pulling fee7ad4 on proxy_install into 55a84ba on master.

Copy link
Contributor

@imobachgs imobachgs left a comment

Choose a reason for hiding this comment

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

It looks good to me, although I think that the logic to handle the proxy configuration should be in the Network service (instead of the Manager). However, let's consider the current implementation good enough.

Please add the entry to the changes file.

@teclator
Copy link
Contributor Author

It looks good to me, although I think that the logic to handle the proxy configuration should be in the Network service (instead of the Manager). However, let's consider the current implementation good enough.

Please add the entry to the changes file.

I agree, lets do it separately by now and then integrate it in the network service implementing it completely in Rust.

@imobachgs
Copy link
Contributor

BTW, the integration tests are failing, but it is a known (and already) fixed issue. It would be fine after you merge.

@teclator
Copy link
Contributor Author

Rebased with master and added changelog

@teclator teclator merged commit f992c17 into master Aug 28, 2023
12 of 14 checks passed
@teclator teclator deleted the proxy_install branch August 28, 2023 08:08
This was referenced Sep 26, 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.

3 participants