You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "uranusjr (via GitHub)" <gi...@apache.org> on 2023/11/28 05:06:32 UTC

[PR] Ignore failed chown during breeze env config [airflow]

uranusjr opened a new pull request, #35905:
URL: https://github.com/apache/airflow/pull/35905

   In an edge case, a host's user or group ID may cause the chown method to not succeed if they have special meanings in the container. This does not necessarily make Breeze unfunctional (depending on the actual permission in the host), so it may be better to simply skip the chown instead of failing the entire command and cause Breeze to exit.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Ignore failed chown during breeze env config [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #35905:
URL: https://github.com/apache/airflow/pull/35905#issuecomment-1829542433

   Nice. Yeah. Some Windows FS does not support chown if you have the Windows FS mounted. It does not bring full Windows breeze functionality in, but in some cases it might help. 
   
   Suggestion @uranusjr : If we do want to make Breeze to work on windows - maybe a  good idea to add  a separate CI job on Windows machines where we run installation and `breeze shell ` command on Windows. This would prevent any future windows-related failures. We run plenty of breeze commands in our CI to prevent similar things creeping in,
   
   Was i right that It was Windows? What's your setup when you experienced it?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Ignore failed chown during breeze env config [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #35905:
URL: https://github.com/apache/airflow/pull/35905#issuecomment-1829608749

   Just want to get to the bottom of the weird UID - I think that could have actually be the case with ROOTLESS_DOCKER - because there there is user remapping going on under-the hood - and depending on the rootles docker solution (there are slight differences in podman, docker-desktop configured in rootless mode and colima) 
   
   The important thing in this case is that we do not have to do two thing in ROOTLESS_DOCKER:
   
   1) we do not have to change ownership of files (and even cannot because HOST_UID has different meaning in-docker than out-docker (the UIDs are remapped) 
   
   2) we do not have to fix ownership of created files, because the user we run with by default is usually remapped from the original user from the host, so we should not even attempt to fix the ownership of the files when exiting the container.
   
   
   We are allready doing the lattter in breeze, but likely I will have to take a look if there are other places where it we might have similar problems. Rootless docker is gaining populrity as this is quite a bit more secure way of running docker containers and many of the solution (no docker-desktop yet) start to switch to it by default, so we might want to make sure it is the first-class-citizen for Breeze as well.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Ignore failed chown during breeze env config [airflow]

Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on PR #35905:
URL: https://github.com/apache/airflow/pull/35905#issuecomment-1829574008

   This specific case is not Windows but just the user has a weird ID that can’t be chown-ed. I didn’t look too close into it tbh and it’s likely possible to fix this from another angle, but since the chown is not strictly necessary anyway I opted for the easy way out of the situation.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Ignore failed chown during breeze env config [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #35905:
URL: https://github.com/apache/airflow/pull/35905#issuecomment-1829648930

   More complete fix in https://github.com/apache/airflow/pull/35917 (I left the `|| true` as well, because it makes no harm and possiby handles other cases) - but I documented the rootless docker changing our past decisions about changing ownership inside docker ( I forgot to add it when I added rootles docker in https://github.com/apache/airflow/pull/34537 in September


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Ignore failed chown during breeze env config [airflow]

Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr merged PR #35905:
URL: https://github.com/apache/airflow/pull/35905


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Ignore failed chown during breeze env config [airflow]

Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on PR #35905:
URL: https://github.com/apache/airflow/pull/35905#issuecomment-1830473183

   It could be. Unfortunatly I don’t have access to the machine anymore, but the setup is not a very trivially fresh Docker Desktop installation but with a bunch of configs mixed in.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Ignore failed chown during breeze env config [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #35905:
URL: https://github.com/apache/airflow/pull/35905#issuecomment-1829589071

   > This specific case is not Windows but just the user has a weird ID that can’t be chown-ed. I didn’t look too close into it tbh and it’s likely possible to fix this from another angle, but since the chown is not strictly necessary anyway I opted for the easy way out of the situation.
   
   Ah I see. I thought it was on the Windows FS - I saw similar problems reported before -  because there `chown`  would fail as Windows FS is not POSIX and does not support chown, Interesting case with the ID though.. BTW. We also have another case (i.e. rootless docker) and maybe that could be the case as well (in both case the solution will work however).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Ignore failed chown during breeze env config [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #35905:
URL: https://github.com/apache/airflow/pull/35905#issuecomment-1829591502

   For the latter case we also have `DOCKER_IS_ROOTLESS` env var passed to in-container (just in case it is the rootless docker case and some other things will not work)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org