launchpad-reviewers team mailing list archive

[Simone Pelosi <mp+482529@xxxxxxxxxxxxxxxxxx>]

Thank you so much for your contribution <3 It looks good to me: the following MP is handling properly all the *craft builds but it's missing support for OCI builds and Livefs. 

Regarding the OCI builds we are handling the http_proxy/https_proxy variable differently, we are propagating them as build args:
```
        if self.args.proxy_url:
            for var in ("http_proxy", "https_proxy"):
                args.extend(["--build-arg", f"{var}={self.args.proxy_url}"])
```
I guess that _add_docker_engine_proxy_settings function should be modified accordingly.

For the Livefs is the same since it's not using the BuilderProxyOperationMixin class to build the proxy.
```
            if self.args.http_proxy:
                proxy_dict = {
                    "http_proxy": self.args.http_proxy,
                    "LB_APT_HTTP_PROXY": self.args.http_proxy,
                }
```

Moreover, I want to ask you an example for which we need the uppercase env variable (Just curiosity :P)
If you need help with the changes, I will be really happy to help you.

Please don't forget to update the test suite properly (for Livefs and OCI builds).
-- 
https://code.launchpad.net/~jnsgruk/launchpad-buildd/+git/launchpad-buildd/+merge/482529
Your team Launchpad code reviewers is requested to review the proposed merge of ~jnsgruk/launchpad-buildd:proxy-vars-uppercase into launchpad-buildd:master.