You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/07/23 12:46:16 UTC

[GitHub] [airflow] Hasan-J opened a new pull request, #25251: Make codespace run successfully

Hasan-J opened a new pull request, #25251:
URL: https://github.com/apache/airflow/pull/25251

   Trying to make codespaces work.
   
   I faced 2 issues with that, fixed by the first 2 commits respectively.
   
   The issue with `local.yml` compose file was that it kept failing the build because of the mount source paths, if they don't exist on the host `Error response from daemon: invalid mount config for type "bind": bind source path does not exist`.
   
   I did some troubleshooting and the only thing I could come up with is that the docker-compose version is not the culprit here. I used the bootstrap image that codspaces use and were able to successfully bind mount non-existing dirs, so it could be something with the node app they use to get everything up and running? 
   
   Side inquiry, I was initially confused as to what's the actual purpose for spinning up a codespace instance. Is it so we could mainly have testing env similar to CI and do only breeze tasks after you have done some changes somewhere else? or is it supposed to be a complete development env (similar to local on your machine) so you can create local virtual environmensts and launch different breeze configs? This question comes up after I realized that in the codespace instance you'd have:
   - `/opt/airflow` containing only the needed mounts from `local.yml`, with no `.git` dir.
   - `/workspaces/airflow` with the full code base and `.git` dir
   
   > Note: The changes in local.yml are just temp and only to get the codespace instance to run, I'm hoping to discuss it further in order to find a better solution


-- 
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

[GitHub] [airflow] Hasan-J commented on pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
Hasan-J commented on PR #25251:
URL: https://github.com/apache/airflow/pull/25251#issuecomment-1193153169

   I share the same disappointment with you indeed and I'm only on the beta too.
   
   Originally I wanted to avoid setting up my local machine, so I went the route of dockerizing everything, even my development environments to keep my sanity. Codespaces with their current "free" state are enough for specific types of contributions as you explained but what do you think about supporting Microsoft dev containers?
   
   First, I thought this would be impossible because you would need to combine Codespaces and dev-containers configurations
   but with the way codespace instance launching works we could separate them into `.devcontainer/.devcontainer-codespaces.json` & keep the `.devcontainer/devcontainer.json` for the local devcontainer workflow.
   
   This will allow power users to keep working the same way only without affecting their local machine binaries (only talking about external dependencies for the local virtual env setup here)
   
   Is it worth 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

[GitHub] [airflow] Hasan-J commented on a diff in pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
Hasan-J commented on code in PR #25251:
URL: https://github.com/apache/airflow/pull/25251#discussion_r928142383


##########
scripts/ci/docker-compose/local.yml:
##########
@@ -27,12 +27,6 @@ services:
     # tests (such as logs)
     volumes:
       # START automatically generated volumes from VOLUMES_FOR_SELECTED_MOUNTS in docker_command_utils.py
-      - type: bind

Review Comment:
   This is great, I will try with the `local-all-sources.yml` but it also contains the same breaking mounts as `local.yml`
   
         - ../../../.bash_aliases:/root/.bash_aliases:cached
         - ../../../.bash_history:/root/.bash_history:cached
         - ../../../.inputrc:/root/.inputrc:cached
         - ../../../tmp:/tmp:cached



-- 
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

[GitHub] [airflow] potiuk commented on pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #25251:
URL: https://github.com/apache/airflow/pull/25251#issuecomment-1198326220

   AH.... Yeah... :facepalm:  That's a bummer. I forgot that some time ago we STOPPED publishing the image - we only publish a cache the image can be build from. Which actually I might want to revisit - because for those kinds of usages, the "latest" image would be best ..... Another PR in the making there :)


-- 
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

[GitHub] [airflow] Hasan-J commented on a diff in pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
Hasan-J commented on code in PR #25251:
URL: https://github.com/apache/airflow/pull/25251#discussion_r928147125


##########
scripts/ci/docker-compose/local.yml:
##########
@@ -27,12 +27,6 @@ services:
     # tests (such as logs)
     volumes:
       # START automatically generated volumes from VOLUMES_FOR_SELECTED_MOUNTS in docker_command_utils.py
-      - type: bind

Review Comment:
   Did you try it? I haven't tried your suggestions yet.
   
   Because these are only specific to the scenario where you use a dockerfile in your devcontainer.json and not dockercompose file, there are the available options for docker compose https://code.visualstudio.com/docs/remote/devcontainerjson-reference#_docker-compose-specific-properties



-- 
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

[GitHub] [airflow] Hasan-J commented on pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
Hasan-J commented on PR #25251:
URL: https://github.com/apache/airflow/pull/25251#issuecomment-1229021839

   No worries. I see how busy you are XD
   Thank you.


-- 
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

[GitHub] [airflow] potiuk commented on a diff in pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #25251:
URL: https://github.com/apache/airflow/pull/25251#discussion_r928143394


##########
scripts/ci/docker-compose/local.yml:
##########
@@ -27,12 +27,6 @@ services:
     # tests (such as logs)
     volumes:
       # START automatically generated volumes from VOLUMES_FOR_SELECTED_MOUNTS in docker_command_utils.py
-      - type: bind

Review Comment:
   > And we would still have source code in 2 different locations `/opt/airflow` & `/workspaces/airflow`. I don't know what to make of that!
   
   No problem whatsoever. There is no performance penalty if the same folders are mounted in two places. We actually often do it in breeze, it makes things easier.
   
   I do not want to make "/workspaces/folder" influence Airflow's dev env. It's the choice made by DevContainers, but from our point of view this is arbitrary choice of just one dev env approaches and we should not follow it. If we could disable it, it would be great (maybe?) but if not - I am not too worried.



-- 
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

[GitHub] [airflow] Hasan-J commented on pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
Hasan-J commented on PR #25251:
URL: https://github.com/apache/airflow/pull/25251#issuecomment-1193364928

   Hello, I discovered something about how codespaces work as I was trying to use `local-all-sources.yml`.
   
   The codespaces container creation kept failing without any useful logs (just container creation failed), there was something happening inside of it, specifically during the execution of `/entrypoint`. The problem was because it didn't find the sh scripts it needed:
   ```
   Container started
   /entrypoint: line 6: /opt/airflow/scripts/in_container/_in_container_script_init.sh: No such file or directory
   ```
   It looks like the mount was successful but with weird behavior, so I overridden the entrypoint to just sleep and do nothing and it worked I was inside the container, BUT `/opt/airflow` was empty.
   
   I dug a little more in the docs and found this https://code.visualstudio.com/remote/advancedcontainers/add-local-file-mount
   
   ![image](https://user-images.githubusercontent.com/18291326/180656605-88e6b43c-767a-4175-b591-a575fcc71864.png)
   
   Given these limitations:
   - Specifying `workspaceFolder` has no effect, codespaces will open `workspaces/airflow` folder by default.
   - Can't do local bind mounts 
   
   How does working in `workspaces/airflow` only on codespaces sound like? or we could create a symlink at `/opt/airflow`, but it could be inconvenient because you'd end up with opened folder at `workspaces/airflow` and a terminal at `/opt/airflow`.


-- 
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

[GitHub] [airflow] potiuk commented on pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #25251:
URL: https://github.com/apache/airflow/pull/25251#issuecomment-1203023925

   Hm. Actually if this is that complex, there is nothing wrong in mounting airflow to /workspace/airflow. We could - if we want - make sure that all the scripts inside the image are reletive to AIRFLOW_SOURCES - when I checked it, there are maybe a few places where "/opt/airlfow" is specifically mentioned, so we could set AIRFLOW_SOURCES to /workspace/airflow when we start the container (maybe we could also remove sources by mounting empty volume in /opt/airflow or smth). 


-- 
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

[GitHub] [airflow] potiuk commented on a diff in pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #25251:
URL: https://github.com/apache/airflow/pull/25251#discussion_r928143871


##########
scripts/ci/docker-compose/local.yml:
##########
@@ -27,12 +27,6 @@ services:
     # tests (such as logs)
     volumes:
       # START automatically generated volumes from VOLUMES_FOR_SELECTED_MOUNTS in docker_command_utils.py
-      - type: bind

Review Comment:
   Actually I looked it up quickly and we can remove `local-sources-all.yml" altogether and use this
   https://code.visualstudio.com/remote/advancedcontainers/change-default-source-mount to mount airflow sources to "/opt/airflow". 
   
   Issue solved. 



-- 
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

[GitHub] [airflow] potiuk commented on pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #25251:
URL: https://github.com/apache/airflow/pull/25251#issuecomment-1198353239

   PR here @Hasan-J : https://github.com/apache/airflow/pull/25380 - this one should keep the images relatively fresh (they refresh when all tests on main build succeed after merge).
   
   I also just refreshed the latest image - so when you pull it next time the image should be brand fresh. Note that it will not contain YOUR changes - it will be a "main" sources image that your sources will be mounted to :)
   
   All 4 images are pushed now (3.7 - 3.10). 


-- 
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

[GitHub] [airflow] Hasan-J commented on pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
Hasan-J commented on PR #25251:
URL: https://github.com/apache/airflow/pull/25251#issuecomment-1200203640

   Hello
   
   After trying to create a symlink I discovered it's not as straightforward as I thought it would be.
   
   The problem is the image already has the directory `/opt/airflow` and that prevents me from creating the symlink as you can't overwrite a directory, so you have to remove it first. Removing that dir first also doesn't solve the problem because of how the initialization scripts (e.g. `_in_container_script_init`) work. `/opt/airflow` would be removed by the `BREEZE_INIT_COMMAND` in `run_init_script.sh` before `in_container_fix_ownership` function runs for instance and that would break things. 
   
   Another solution would be to also mount the repo to `/opt/airflow` in addition to the original mount which is `/workspaces/airflow`. I looked at the [initializeCommand](https://code.visualstudio.com/docs/remote/devcontainerjson-reference#_lifecycle-scripts) in order to detect the volume name codespaces creates when launching the container:
   
   `.devcontainer/devcontainer.json`
   ```yaml
   ...,
       "initializeCommand": "/workspaces/airflow/scripts/devcontainer_initialize_command.sh"
   }
   ```
   `devcontainer_initialize_command.sh` would detect the volume name and writes it to `.env` file relative to `scripts/ci/docker-compose/devcontainer.yml`
   
   Then use env variable substitution in:
   
   `scripts/ci/docker-compose/devcontainer.yml`
   ```yaml
   ...,
   volumes:
    ...
    - workspace:/opt
   
   volumes:
     workspace:
       external: true
       name: ${DEVCONTAINER_VOLUME}
   ``` 
   
   The only unwanted side-effect of this solution is the new mount entry `- workspace:/opt` it will overwrite the whole `/opt` dir removing the microsoft odbc tool already installed in the image.
   
   ---
   
   Plus, one more annoying thing (but still a bit acceptable I think) is that codespaces' default folder and terminal workdir would still be `/workspaces/airflow`. We can change the terminal default dir:
   
   `.devcontainer/devcontainer.json`
   ```yaml
   ...,
   "settings": {
     "terminal.integrated.cwd": "/opt/airflow"
   },
   ...
   ```
   
   But that's about it. People should be careful when copying full paths (not relative) from the side explorer window.


-- 
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

[GitHub] [airflow] potiuk commented on pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #25251:
URL: https://github.com/apache/airflow/pull/25251#issuecomment-1217932741

   Wooohooo. I will take a look at it later today :)


-- 
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

[GitHub] [airflow] Hasan-J commented on pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
Hasan-J commented on PR #25251:
URL: https://github.com/apache/airflow/pull/25251#issuecomment-1198286823

   Excuse this noob question 🤦‍♂️. Do you have any idea how I can pull the new image? I assumed it will be automatically pulled, since the tag `latest`. But that tag does not exist on [main/ci/python3.7](https://github.com/apache/airflow/pkgs/container/airflow%2Fmain%2Fci%2Fpython3.7/versions). Where does it get pulled from?
    
   I know I don't have the latest because the relevant changes are not there.


-- 
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

[GitHub] [airflow] potiuk commented on a diff in pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #25251:
URL: https://github.com/apache/airflow/pull/25251#discussion_r928158756


##########
scripts/ci/docker-compose/local.yml:
##########
@@ -27,12 +27,6 @@ services:
     # tests (such as logs)
     volumes:
       # START automatically generated volumes from VOLUMES_FOR_SELECTED_MOUNTS in docker_command_utils.py
-      - type: bind

Review Comment:
   That will make it difficult for sharing base/postgres/mysql configs between Breeze when they chamge. Those files evolve and it is best if we keep them in sync. Besides for Postgres/MySQL you need to run separate containers for databases and that is way easier with docker-compose.



-- 
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

[GitHub] [airflow] potiuk commented on a diff in pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #25251:
URL: https://github.com/apache/airflow/pull/25251#discussion_r928138987


##########
.devcontainer/mysql/devcontainer.json:
##########
@@ -1,10 +1,10 @@
 {
     "name": "Apache Airflow - mysql",
     "dockerComposeFile": [
-        "../scripts/ci/docker-compose/devcontainer.yml",
-        "../scripts/ci/docker-compose/local.yml",
-        "../scripts/ci/docker-compose/backend-mysql.yml",
-        "../scripts/ci/docker-compose/devcontainer-mysql.yml"
+        "../../scripts/ci/docker-compose/devcontainer.yml",

Review Comment:
   Ah cool. I am not sure why it does not work but maybe the early version of "muliple codespaces versions" implementation had a different workdir setting :P. Thanks for that one :) 



-- 
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

[GitHub] [airflow] potiuk commented on pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #25251:
URL: https://github.com/apache/airflow/pull/25251#issuecomment-1194146036

   This can be done in "entrypoint_ci.sh". But I am afraid it will have to be done as separate PR to "Add" the feature and refresh images - because the image will you se the "entrypoint_ci.sh" which is already in the image, so we have to make sure the image is rebuild and pushed to ghcr.io before it is used. 


-- 
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

[GitHub] [airflow] Hasan-J commented on pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
Hasan-J commented on PR #25251:
URL: https://github.com/apache/airflow/pull/25251#issuecomment-1221292842

   ops, I deleted by mistake XD


-- 
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

[GitHub] [airflow] Hasan-J closed pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
Hasan-J closed pull request #25251: Make codespace run successfully
URL: https://github.com/apache/airflow/pull/25251


-- 
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

[GitHub] [airflow] potiuk commented on pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #25251:
URL: https://github.com/apache/airflow/pull/25251#issuecomment-1193148196

   > Side inquiry, I was initially confused as to what's the actual purpose for spinning up a codespace instance. Is it so we could mainly have testing env similar to CI and do only breeze tasks after you have done some changes somewhere else? or is it supposed to be a complete development env (similar to local on your machine) so you can create local virtual environmensts and launch different breeze configs? This question comes up after I realized that in the codespace instance you'd have:
   
   The "goal" of codespaces configuration it actually to spin of a local virtualenv. Ideally (that was original purpose why we aded - if you have access to codespaaces and no big local machine and fast network (for example we have a number of contributors in countries where network is not that fast as in most of the Europe and US) they should be able to spin a codespaces machine and run all tests that are normally run with local breeze env.
   
   


-- 
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

[GitHub] [airflow] Hasan-J commented on a diff in pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
Hasan-J commented on code in PR #25251:
URL: https://github.com/apache/airflow/pull/25251#discussion_r928147615


##########
scripts/ci/docker-compose/local.yml:
##########
@@ -27,12 +27,6 @@ services:
     # tests (such as logs)
     volumes:
       # START automatically generated volumes from VOLUMES_FOR_SELECTED_MOUNTS in docker_command_utils.py
-      - type: bind

Review Comment:
    Now for the specific `workspaceFolder` property, I already tried it by setting it to `/opt/airflow` but it didn't work and still put me in the default `/workspaces/airflow` folder :)



-- 
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

[GitHub] [airflow] potiuk commented on a diff in pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #25251:
URL: https://github.com/apache/airflow/pull/25251#discussion_r928157348


##########
scripts/ci/docker-compose/local.yml:
##########
@@ -27,12 +27,6 @@ services:
     # tests (such as logs)
     volumes:
       # START automatically generated volumes from VOLUMES_FOR_SELECTED_MOUNTS in docker_command_utils.py
-      - type: bind

Review Comment:
   Bummer
   



-- 
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

[GitHub] [airflow] potiuk commented on a diff in pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #25251:
URL: https://github.com/apache/airflow/pull/25251#discussion_r928138702


##########
scripts/ci/docker-compose/local.yml:
##########
@@ -27,12 +27,6 @@ services:
     # tests (such as logs)
     volumes:
       # START automatically generated volumes from VOLUMES_FOR_SELECTED_MOUNTS in docker_command_utils.py
-      - type: bind

Review Comment:
   Actually it's quite likely we do not need to use "local" at all for codespaces. This is only needed for local breeze setup when you want to share multiple folders with your container. And the main reason it is not the whole "airflow" folder, is that by mouting whole "source" directory you share all kinds of artifacts - for example .egginfo files whe  you locally run `pip install -e .` - that often produce various errors when you mount them from host. 
   
   In case of codespaces we ONLY have in-container environment, so we do not need to worry about and instead of using local.yml we could create a separate file that would mound whole "airflow" folder (including the .git you mentioned) .
   
   
   Actually even better ... we already have such a file :)
   
   * `local-all-sources.yml` is what we should use instead 
   
   This file is used by breeze when you specify `breeze --mount-source all` which is useful in a numebr of situations (mostly when you need the `.git` folder you mentioned.
   
   So I think better solution will be simply to use `local-all-sources.yml` instead.
   



-- 
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

[GitHub] [airflow] potiuk commented on a diff in pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #25251:
URL: https://github.com/apache/airflow/pull/25251#discussion_r928137342


##########
scripts/ci/docker-compose/local.yml:
##########
@@ -27,12 +27,6 @@ services:
     # tests (such as logs)
     volumes:
       # START automatically generated volumes from VOLUMES_FOR_SELECTED_MOUNTS in docker_command_utils.py
-      - type: bind

Review Comment:
   Those can't be removed. You need to make them work for `breeze` experience. Docker-compose here is NOT EXCLUSIVELY for codespaces. It is there to Power "Breeze" environment, which is the main environment not only for some contributors but also for our CI, So removing those is not good (especially that those entries (as you can see aboeve) are automatically generated based on definition of which volumes shold be mounted (see the comment just above). 
   
   We need to find another way if this faile on devcontainer (but it is strange because it worked before).



-- 
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

[GitHub] [airflow] potiuk commented on pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #25251:
URL: https://github.com/apache/airflow/pull/25251#issuecomment-1198159284

   > PR added here :#25288 - once it is merged, and images refreshed, you should be able to set the `BREEZE_INIT_COMMAND` variable to a command that will be evaluated at Breeze entry time.
   
   It's merged now


-- 
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

[GitHub] [airflow] potiuk merged pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
potiuk merged PR #25251:
URL: https://github.com/apache/airflow/pull/25251


-- 
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

[GitHub] [airflow] Hasan-J commented on pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
Hasan-J commented on PR #25251:
URL: https://github.com/apache/airflow/pull/25251#issuecomment-1214196665

   Right, this would be more straightforward. I have identified the locations where we need to replace `/opt/airflow` with `AIRFLOW_SOURCES` and they include `entrypoint_ci.sh`. I can open another PR to publish them.


-- 
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

[GitHub] [airflow] Hasan-J commented on pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
Hasan-J commented on PR #25251:
URL: https://github.com/apache/airflow/pull/25251#issuecomment-1217526238

   Wait, this is not finished. We can't remove `/opt/airflow` since apache-airflow is installed from there inside the ci image.
   I thought about re-installing from `/workspaces/airflow` on startup, but that could be an anti-pattern to the already built image.
   
   


-- 
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

[GitHub] [airflow] Hasan-J commented on a diff in pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
Hasan-J commented on code in PR #25251:
URL: https://github.com/apache/airflow/pull/25251#discussion_r928147615


##########
scripts/ci/docker-compose/local.yml:
##########
@@ -27,12 +27,6 @@ services:
     # tests (such as logs)
     volumes:
       # START automatically generated volumes from VOLUMES_FOR_SELECTED_MOUNTS in docker_command_utils.py
-      - type: bind

Review Comment:
    Now for the specific `workspaceFolder` property, I already tried it by setting it to `/opt/airflow` but it didn't work and still put me in the default `/workspaces/airflow` folder :)
   
   EDIT: I just noticed that this property is not yet supported on codespaces yet. That note was listed under the dockerfile specific properties though, but I think it's safe to assume that it does not work for docker-compose too.



-- 
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

[GitHub] [airflow] potiuk commented on a diff in pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #25251:
URL: https://github.com/apache/airflow/pull/25251#discussion_r928138017


##########
scripts/ci/docker-compose/local.yml:
##########
@@ -27,12 +27,6 @@ services:
     # tests (such as logs)
     volumes:
       # START automatically generated volumes from VOLUMES_FOR_SELECTED_MOUNTS in docker_command_utils.py
-      - type: bind

Review Comment:
   > Note: The changes in local.yml are just temp and only to get the codespace instance to run, I'm hoping to discuss it further in order to find a better solution
   
   Ah, I only saw that now. Yep. We need to find another way.



-- 
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

[GitHub] [airflow] potiuk commented on pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #25251:
URL: https://github.com/apache/airflow/pull/25251#issuecomment-1194146356

   I can add such PR in a moment.


-- 
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

[GitHub] [airflow] potiuk commented on pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #25251:
URL: https://github.com/apache/airflow/pull/25251#issuecomment-1194171485

   PR added here: https://github.com/apache/airflow/pull/25251 - once it is merged, and images refreshed, you should be able to set the `BREEZE_INIT_COMMAND` variable to a command that will be evaluated at Breeze entry time.


-- 
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

[GitHub] [airflow] Hasan-J commented on a diff in pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
Hasan-J commented on code in PR #25251:
URL: https://github.com/apache/airflow/pull/25251#discussion_r928158406


##########
scripts/ci/docker-compose/local.yml:
##########
@@ -27,12 +27,6 @@ services:
     # tests (such as logs)
     volumes:
       # START automatically generated volumes from VOLUMES_FOR_SELECTED_MOUNTS in docker_command_utils.py
-      - type: bind

Review Comment:
   Maybe switch devcontainer files from using docker-compose to dockerfile/image!



-- 
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

[GitHub] [airflow] Hasan-J commented on a diff in pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
Hasan-J commented on code in PR #25251:
URL: https://github.com/apache/airflow/pull/25251#discussion_r928142537


##########
scripts/ci/docker-compose/local.yml:
##########
@@ -27,12 +27,6 @@ services:
     # tests (such as logs)
     volumes:
       # START automatically generated volumes from VOLUMES_FOR_SELECTED_MOUNTS in docker_command_utils.py
-      - type: bind

Review Comment:
   And we would still have source code in 2 different locations `/opt/airflow` & `/workspaces/airflow`. I don't know what to make of that!



-- 
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

[GitHub] [airflow] potiuk commented on a diff in pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #25251:
URL: https://github.com/apache/airflow/pull/25251#discussion_r928143936


##########
scripts/ci/docker-compose/local.yml:
##########
@@ -27,12 +27,6 @@ services:
     # tests (such as logs)
     volumes:
       # START automatically generated volumes from VOLUMES_FOR_SELECTED_MOUNTS in docker_command_utils.py
-      - type: bind

Review Comment:
   Both issues solved :)



-- 
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

[GitHub] [airflow] Hasan-J commented on pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
Hasan-J commented on PR #25251:
URL: https://github.com/apache/airflow/pull/25251#issuecomment-1217929721

   I finally found a sane workaround to the previously discussed limitations. With these last changes, we can work normally inside /workspaces/airflow while respecting the ci image state by not drastically changing 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

[GitHub] [airflow] potiuk commented on pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #25251:
URL: https://github.com/apache/airflow/pull/25251#issuecomment-1217932946

   Congrats for the persistency !


-- 
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

[GitHub] [airflow] Hasan-J commented on pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
Hasan-J commented on PR #25251:
URL: https://github.com/apache/airflow/pull/25251#issuecomment-1193365548

   Side note: I think the reason bind mounting doesn't work is related to the workflow of the tools/services that work with devcontainer.json configurations. In a scenario where the source code is not local, they end up creating a new docker volume attaching it to a temp bootstrapping container, then cloning the source code inside. So if you have local binds in your custom devcontainer.json it ends up with the described weird behavior because you can't bind mount from inside a container into another one! That's just my analysis.


-- 
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

[GitHub] [airflow] potiuk commented on a diff in pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #25251:
URL: https://github.com/apache/airflow/pull/25251#discussion_r928143030


##########
scripts/ci/docker-compose/local.yml:
##########
@@ -27,12 +27,6 @@ services:
     # tests (such as logs)
     volumes:
       # START automatically generated volumes from VOLUMES_FOR_SELECTED_MOUNTS in docker_command_utils.py
-      - type: bind

Review Comment:
   > This is great, I will try with the `local-all-sources.yml` but it also contains the same breaking mounts as `local.yml`
   
   Those mounts are not breaking. Bind mounts are very different from those "old style" mounts. The "old style" mounts will automatically create missing "sources" as empty dirs. Not PERFECT for local development, but good enough for codespaces.



-- 
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

[GitHub] [airflow] potiuk commented on pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #25251:
URL: https://github.com/apache/airflow/pull/25251#issuecomment-1193154151

   > Is it worth it?
   
   I am not in Microsoft Ecosystem (I  use IntelliJ as daily env) I do not know much about MS devcontainers) but I believe Codespaces is largely about that. But if it works nicely with VSCode and you could test and make it works for both - it's certainly worth 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

[GitHub] [airflow] potiuk commented on pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #25251:
URL: https://github.com/apache/airflow/pull/25251#issuecomment-1193149731

   Cool. Glad to see that someone is actually trying to use codespaces. 
   
   Just a side comment - the big bummer with codespaces that even if it had public "free" version, it is only available to paying customers (and those who signed to public beta like me). This actually heavy deacreases it's usability. I was **really** hoping we could use it as a way for underrepresented people in the OSS - those who cannot afford big machines and high-speed networks. Or those who have Windows (we do not have great support for Windows users yet) and woudl like to make a quick fix.
   
   It would seem PERFECT for this if not the fact that those contributors are exactly those who cannot affort paying to get codespaces access. For me that was one of the greatest disappointment in GitHub when they announced that it's going to be only available for paying customers. This is why codespaces support does not get much love in Airflow - I personally see codespaces as "good enough" for most users who are occasional contributors, and who could simply use it as "hey let's get environment up and running to run those tests". No machine, no network, no setup. With the "size of machines" and remote access roundtrip times and generally latency, it's not good enough for power, regular users who can spend a little more time on setting up development environment of their own and can affort to set-up a good local dev env.
   
   That's why Codespaces support is a little underdog for us.


-- 
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

[GitHub] [airflow] potiuk commented on a diff in pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #25251:
URL: https://github.com/apache/airflow/pull/25251#discussion_r928138702


##########
scripts/ci/docker-compose/local.yml:
##########
@@ -27,12 +27,6 @@ services:
     # tests (such as logs)
     volumes:
       # START automatically generated volumes from VOLUMES_FOR_SELECTED_MOUNTS in docker_command_utils.py
-      - type: bind

Review Comment:
   Actually it's quite likely we do not need to use "local.yml" at all for codespaces. This is only needed for local breeze setup when you want to share multiple folders with your container. And the main reason it is not the whole "airflow" folder, is that by mouting whole "source" directory you share all kinds of artifacts - for example .egginfo files whe  you locally run `pip install -e .` - that often produce various errors when you mount them from host. 
   
   In case of codespaces we ONLY have in-container environment, so we do not need to worry about and instead of using local.yml we could create a separate file that would mound whole "airflow" folder (including the .git you mentioned) .
   
   
   Actually even better ... we already have such a file :)
   
   * `local-all-sources.yml` is what we should use instead 
   
   This file is used by breeze when you specify `breeze --mount-source all` which is useful in a numebr of situations (mostly when you need the `.git` folder you mentioned.
   
   So I think better solution will be simply to use `local-all-sources.yml` instead.
   



-- 
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

[GitHub] [airflow] potiuk commented on pull request #25251: Make codespace run successfully

Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #25251:
URL: https://github.com/apache/airflow/pull/25251#issuecomment-1194143497

   Making symlink to "/workspace/airflow" sounds lika a good idea.


-- 
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