Build: hardcode the Docker username for now

[humitos]

This is a quick fix for a hotfix.

RTD_DOCKER_USER is docs:docs so we can't use it inside the path.

Closes #10171

[benjaoming]

I think you could simplify it as proposed?

[benjaoming]

This one is not part of the else clause?

[humitos]

Nope, this is only required for the new Ubuntu images since these use asdf and this path is where asdf is installed.

[ericholscher]

Can we just use $HOME or similar, instead of adding more hacks? I'm fine with this though for now, assuming we've tested it and it works.

[benjaoming]

The hard-coding makes sense, probably there should be a follow-up issue for this?

Edit: If posting a follow-up issue, it could also note that this area of the code has 0% test coverage, in case its possible to put together a test.

[ericholscher]

This is the downside of trying to set explicit paths, instead of letting the container define them. Lots of complexity abounds when we start trying to define things like this outside of the container environment.

I'm 👍 on shipping this as a hotfix, but we'll likely get bit by this again in the future, so might be worth trying to see if there's a way to avoid setting these paths.

[ericholscher]

Can we just use $HOME or similar, instead of adding more hacks? I'm fine with this though for now, assuming we've tested it and it works.

[humitos]

This is the downside of trying to set explicit paths, instead of letting the container define them.

I prefer to set the environment exactly as we want from the application than rely on the darkness of the things defined in the Dockerfile used to create the image. This approach gives a lot more control and allows us to solve issues without having to rebuild the image 👍🏼

[benjaoming]

@humitos I think it might be tedious to use $HOME and since this is getting cleaned up later anyways, perhaps just leave a code comment with @ericholscher 's suggestion?

[humitos]

@benjaoming @ericholscher I've already tried that and I wasn't able to make $HOME (or any other variable) to expand when using the environment= argument in the DockerAPI call. Not sure why that happens 🤷🏼

The .replace() line is only used for local Docker development, so not a big issue for now.

[benjaoming]

@humitos that's exactly the result I'd fear that you would run into, so best saved for later 👍 and no, not a big issue.

[benjaoming]

AFAICT, this is ready?

[humitos]

@benjaoming yes; I'll do some QA on production in a moment before merging. If everything works as expected, I'll merge and deploy it today.

[humitos]

I QAed this PR with 15 builds (different scenarios) on production and it did work. I'm merging and moving forward here.

[stsewd]

Another solution is to first get the current PATH, and then append the new ones to that.