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 2020/08/18 15:02:48 UTC

[GitHub] [airflow] potiuk opened a new pull request #10380: Added Documentation for future CI with mermaid sequence diagrams

potiuk opened a new pull request #10380:
URL: https://github.com/apache/airflow/pull/10380


   Part of #10368
   
   ---
   **^ Add meaningful description above**
   
   Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
   In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
   In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
   In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
   


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

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



[GitHub] [airflow] potiuk commented on pull request #10380: Updated Documentation for the CI with mermaid sequence diagrams

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


   > I am definitely against have a CI-CD for a single tool, which if it breaks for whatever reason is a code debt we should not have.
   
   It only runs in CI-CD if the source mermaid files change. We keep hashes of the files and no mermaid will be run when then .mermaid files are not touched.


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

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



[GitHub] [airflow] potiuk commented on pull request #10380: Updated Documentation for the CI with mermaid sequence diagrams

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


   Latest additions and rebasing to latest master. I would love to get this one reviewed - it's purely documentation of the CI setup we have now, - but it includes mermaid-generated images, and I believe @kaxil and @mik-laj have some doubts about it, so I would love to hear more about 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.

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



[GitHub] [airflow] kaxil edited a comment on pull request #10380: Updated Documentation for the CI with mermaid sequence diagrams

Posted by GitBox <gi...@apache.org>.
kaxil edited a comment on pull request #10380:
URL: https://github.com/apache/airflow/pull/10380#issuecomment-679216493


   There are integrations available:
   
   - https://drawio-app.com/create-mermaid-diagrams-in-draw-io/ 
   - https://github.com/nopeslide/drawio_mermaid_plugin
   
   
   Or we could just have the png and steps to reproduce like copy/paste text to https://mermaid-js.github.io/mermaid-live-editor/


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

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



[GitHub] [airflow] potiuk commented on pull request #10380: Updated Documentation for the CI with mermaid sequence diagrams

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


   It's there. Standalone bash `pre-commit` script that will only do anything if any of the .mermaid files changed vs. hashes stored next to them. 
   
   That "anything" is installing mermaid in a separate node virtualenv (in ,build/mermaid  folder - the ".build" folder is already all but ignored due to being used by breeze). It will install mermaid there, first time when it is run and keep on reusing it every next time it is run. 
   
   If there is no change in those files or if you have no npm installed, the pre-commit will silently succeed.  If you do not touch any of those files, it's a no-op - both for the developers and for the CI. First of all pre-commit will be skipped if those files are not changed, secondly - even if you run pre-commit with `--all-files` those files will be skipped as the .md5 hashes will not change for them. And even if they do, but you have npm, they will be silently ignored.
   
   I even made sure not to commit the mermaid-config.json - it is dynamically generated by the self-contained bash script. I expect this script to never change (unless we want to update colors/fonts etc. in the generated mermaid files).


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

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



[GitHub] [airflow] feluelle commented on a change in pull request #10380: Updated Documentation for the CI with mermaid sequence diagrams

Posted by GitBox <gi...@apache.org>.
feluelle commented on a change in pull request #10380:
URL: https://github.com/apache/airflow/pull/10380#discussion_r474459881



##########
File path: CI.rst
##########
@@ -188,32 +578,104 @@ The issues that gets updated are different for different branches:
 
 * master: `Quarantine tests master <https://github.com/apache/airflow/issues/10118>`_
 * v1-10-stable: `Quarantine tests v1-10-stable <https://github.com/apache/airflow/issues/10127>`_
-* v1-10-test: `Quarantine tests v1-10-test <hhttps://github.com/apache/airflow/issues/10128>`_
-
-+---------------------------+----------------------------------------------------------------------------------------------------------------+------------------------------------+---------------------------------+----------------------------------------------------------------------+
-| Job                       | Description                                                                                                    | Pull Request Run                   | Direct Push/Merge Run           | Scheduled Run                                                        |
-+===========================+================================================================================================================+====================================+=================================+======================================================================+
-| Cancel previous workflow  | Cancels the previously running workflow run if there is one running                                            | Yes                                | Yes                             | Yes *                                                                |
-+---------------------------+----------------------------------------------------------------------------------------------------------------+------------------------------------+---------------------------------+----------------------------------------------------------------------+
-| Trigger tests             | Checks if tests should be triggered                                                                            | Yes                                | Yes                             | Yes *                                                                |
-+---------------------------+----------------------------------------------------------------------------------------------------------------+------------------------------------+---------------------------------+----------------------------------------------------------------------+
-| Quarantined tests         | Those are tests that are flaky and we need to fix them                                                         | Yes (if tests-triggered)           | Yes (Updates quarantine issue)  | Yes * (updates quarantine issue)                                     |
-+---------------------------+----------------------------------------------------------------------------------------------------------------+------------------------------------+---------------------------------+----------------------------------------------------------------------+
-
-Cancel other workflow runs workflow
------------------------------------
-
-This workflow is run only on schedule (every 5 minutes) it's only purpose is to cancel other running
-``CI Build`` workflows if important jobs failed in those runs. This is to save runners for other runs
-in case we know that the build will not succeed anyway without some basic fixes to static checks or
-documentation - effectively implementing missing "fail-fast" (on a job level) in Github Actions
-similar to fail-fast in matrix strategy.
-
-The jobs that are considered as "fail-fast" are:
-
-* Static checks
-* Docs
-* Prepare Backport packages
-* Helm tests
-* Build Prod Image
-* TTest OpenAPI client gen
+* v1-10-test: `Quarantine tests v1-10-test <https://github.com/apache/airflow/issues/10128>`_
+
+Force sync master from apache/airflow
+-------------------------------------
+
+This is manually triggered workflow (via GitHub UI manual run) that should only be run in GitHub forks.
+When triggered, it will force-push the "apache/airflow" master to the fork's master. It's the easiest
+way to sync your fork master to the Apache Airflow's one.
+
+Naming conventions for stored images
+====================================
+
+The images produced during the CI builds are stored in the
+`GitHub Registry <https://github.com/apache/airflow/packages>`_
+
+The images are stored with both "latest" tag (for last master push image that passes all the tests as well
+with the tags indicating the origin of the image.
+
+The image names follow the patterns:
+
++--------------+----------------------------+--------------------------------+--------------------------------------------------------------------------------------------+
+| Image        | Name pattern               | Tag for format                 | Comment                                                                                    |
++==============+============================+================================+============================================================================================+
+| Python image | python                     | <X.Y>-slim-buster-<RUN_ID>     | Base python image used by both production and CI image.                                    |
+|              |                            | <X.Y>-slim-buster-<COMMIT_SHA> | Python maintainer release new versions of those image with security fixes every few weeks. |
++--------------+----------------------------+--------------------------------+--------------------------------------------------------------------------------------------+
+| CI image     | <BRANCH>-python<X.Y>-ci    | <RUN_ID>                       | CI image - this is the image used for most of the tests.                                   |
+|              |                            | <COMMIT_SHA>                   |                                                                                            |
++--------------+----------------------------+--------------------------------+--------------------------------------------------------------------------------------------+
+| PROD Build   | <BRANCH>-python<X.Y>-build | <RUN_ID>                       | Production Build image - this is the "build" segment of production image.                  |
+| image        |                            | <COMMIT_SHA>                   | It contains build-essentials and all necessary packages to install PIP packages.           |
++--------------+----------------------------+--------------------------------+--------------------------------------------------------------------------------------------+
+| PROD image   | <BRANCH>-python<X.Y>       | <RUN_ID>                       | Production image. This is the actual production image - optimized for size.                |
+|              |                            | <COMMIT_SHA>                   | It contains only compiled libraries and minimal set of dependencies to run Airflow.        |
++--------------+----------------------------+--------------------------------+--------------------------------------------------------------------------------------------+
+
+* <BRANCH> might be either "master" or "v1-10-test"
+* <X.Y> - Python version (Major + Minor). For "master" it should be in ["3.6", "3.7", "3.8"]. For
+  v1-10-test it should be in ["2.7", "3.5", "3.6". "3.7", "3.8"].
+* <RUN_ID> - Github Actions RUN_ID. You can get it from CI action job outputs (run id is printed in
+  logs and displayed as part of the step name. All PRs belong to some RUN_ID and this way you can
+  pull the very exact version of image used in that RUN_ID
+* <COMMIT_SHA> - for images that get merged to "master" of "v1-10-test" the images are also tagged
+  with the commit SHA of that particular commit. This way you can easily find the image that was used
+  for testing for that "master" or "v1-10-test" test run.
+
+Reproducing CI Runs locally
+===========================
+
+Since we store images from every CI run, you should be able easily reproduce any of the CI build problems
+locally. You can do it by pulling and using the right image and running it with the right docker command,
+For example knowing that the CI build had 210056909 RUN_ID (you can find it from GitHub CI logs):
+
+.. code-block:: bash
+
+  docker pull docker.pkg.github.com/apache/airflow/master-python3.6-ci:210056909
+
+  docker run -it docker.pkg.github.com/apache/airflow/master-python3.6-ci:210056909
+
+
+But you usually need to pass more variables amd complex setup if you want to connect to a database or
+enable some integrations. Therefore it is easiest to use `Breeze <BREEZE.rs>`_ for that. For example if

Review comment:
       ```suggestion
   enable some integrations. Therefore it is easiest to use `Breeze <BREEZE.rst>`_ for that. For example if
   ```

##########
File path: CI.rst
##########
@@ -17,6 +17,10 @@
 
 .. contents:: :local:
 
+NOTE! This is the target architecture we are aspiring to. We have a
+`<work in progress PR https://github.com/apache/airflow/pull/10368>`_ which implements this
+architecture but the documentation generation and plan is merged separately as "future" change.
+

Review comment:
       This is not up to date anymore, is it?

##########
File path: images/ci/pull_request_ci_flow.mermaid
##########
@@ -0,0 +1,108 @@
+%% Licensed to the Apache Software Foundation (ASF) under one
+%% or more contributor license agreements.  See the NOTICE file
+%% distributed with this work for additional information
+%% regarding copyright ownership.  The ASF licenses this file
+%% to you under the Apache License, Version 2.0 (the
+%% "License"); you may not use this file except in compliance
+%% with the License.  You may obtain a copy of the License at
+%%
+%%   http://www.apache.org/licenses/LICENSE-2.0
+%%
+%% Unless required by applicable law or agreed to in writing,
+%% software distributed under the License is distributed on an
+%% "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+%% KIND, either express or implied.  See the License for the
+%% specific language governing permissions and limitations
+%% under the License.
+
+sequenceDiagram
+    Contributor-->>Fork: Submit PR

Review comment:
       ```suggestion
       Contributor -->> Fork: Submit PR
   ```




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

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



[GitHub] [airflow] potiuk commented on pull request #10380: Updated Documentation for the CI with mermaid sequence diagrams

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


   Ad discussed in #10368 - I am going to merge that one after the code is reviewed/fixed/merged


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

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



[GitHub] [airflow] potiuk commented on pull request #10380: Updated Documentation for the CI with mermaid sequence diagrams

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


   Udpated the first review by @feluelle .


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

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



[GitHub] [airflow] kaxil commented on pull request #10380: Updated Documentation for the CI with mermaid sequence diagrams

Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #10380:
URL: https://github.com/apache/airflow/pull/10380#issuecomment-679222832


   Maintaining a separate node file just for mermaid makes 0 sense


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

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



[GitHub] [airflow] potiuk merged pull request #10380: Updated Documentation for the CI with mermaid sequence diagrams

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


   


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

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



[GitHub] [airflow] potiuk edited a comment on pull request #10380: Updated Documentation for the CI with mermaid sequence diagrams

Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #10380:
URL: https://github.com/apache/airflow/pull/10380#issuecomment-679254618


   It's there. Standalone bash `pre-commit` script that will only do anything if any of the .mermaid files changed vs. hashes stored next to them. 
   
   That "anything" is installing mermaid in a separate node virtualenv (in ,build/mermaid  folder - the ".build" folder is already all but ignored due to being used by breeze). It will install mermaid there, first time when it is run and keep on reusing it every next time it is run. 
   
   If there is no change in those files or if you have no npm installed, the pre-commit will silently succeed.  If you do not touch any of those files, it's a no-op - both for the developers and for the CI. First of all pre-commit will be skipped if those files are not changed, secondly - even if you run pre-commit with `--all-files` those files will be skipped as the .md5 hashes will not change for them. And even if they do, but you have no npm, they will be silently ignored.
   
   I even made sure not to commit the mermaid-config.json - it is dynamically generated by the self-contained bash script. I expect this script to never change (unless we want to update colors/fonts etc. in the generated mermaid files).


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

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



[GitHub] [airflow] potiuk commented on pull request #10380: Updated Documentation for the CI with mermaid sequence diagrams

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


   I already plan some changes in those diragras - for example when I implement #10507 -  there will be quite some changes there and simply changing that in text is so much easier than having to do all those manual steps. 


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

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



[GitHub] [airflow] kaxil commented on pull request #10380: Updated Documentation for the CI with mermaid sequence diagrams

Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #10380:
URL: https://github.com/apache/airflow/pull/10380#issuecomment-679318428


   CI is failing, maybe needs a rebase


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

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



[GitHub] [airflow] kaxil commented on pull request #10380: Updated Documentation for the CI with mermaid sequence diagrams

Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #10380:
URL: https://github.com/apache/airflow/pull/10380#issuecomment-679222400


   You don't see the logic behind it because you are not reasoning for the amount of code that needs to be maintained


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

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



[GitHub] [airflow] potiuk commented on pull request #10380: Updated Documentation for the CI with mermaid sequence diagrams

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


   > > Are the package.json and the files for building mermaid diagrams in some other PR? I remember seeing them
   > 
   > Just looking at that. I think they are lost on rebase :)
   
   I will restore them in the way I think they can work without package.json. 


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

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



[GitHub] [airflow] potiuk commented on pull request #10380: Updated Documentation for the CI with mermaid sequence diagrams

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


   Rebased after merging #10368 - I think it is ready for review and very accurately describes what we have in CI 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.

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



[GitHub] [airflow] potiuk commented on a change in pull request #10380: Updated Documentation for the CI with mermaid sequence diagrams

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #10380:
URL: https://github.com/apache/airflow/pull/10380#discussion_r474708503



##########
File path: CI.rst
##########
@@ -17,6 +17,10 @@
 
 .. contents:: :local:
 
+NOTE! This is the target architecture we are aspiring to. We have a
+`<work in progress PR https://github.com/apache/airflow/pull/10368>`_ which implements this
+architecture but the documentation generation and plan is merged separately as "future" change.
+

Review comment:
       Correct @feluelle ! Thanks for looking into 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.

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



[GitHub] [airflow] potiuk commented on pull request #10380: Added Documentation for future CI with mermaid sequence diagrams

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


   This is purely a documentation change describeing the future CI architecture as well adding mermaid support for generation of sequence diagrams so that it is easier to review.


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

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



[GitHub] [airflow] potiuk edited a comment on pull request #10380: Updated Documentation for the CI with mermaid sequence diagrams

Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #10380:
URL: https://github.com/apache/airflow/pull/10380#issuecomment-679219467


   > https://drawio-app.com/create-mermaid-diagrams-in-draw-io/ -> There is an integration that is already available
   
   So you basically say you want to still keep the sources of mermaid but rather than doing `git commit` and have them automatically generated you want me to always go to draw,io, copy&paste the code, generate the image, download them, replace the images, commit (and not forget to copy back the sources if I changed them in draw,io?) 
   
   Because that's the alternative. Currently the only thing you need to do is to commit the sources and the images will be generated autopmatically.
   
   This is like forcing me to do those manual steps where simple (literally 20 lines of integration code that will never change) can do it for me. I do not really see the logic behind 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.

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



[GitHub] [airflow] kaxil commented on pull request #10380: Updated Documentation for the CI with mermaid sequence diagrams

Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #10380:
URL: https://github.com/apache/airflow/pull/10380#issuecomment-679222035


   I am definitely against have a CI-CD for a single tool, which if it breaks for whatever reason is a code debt we should not have.
   
   These can be easily replaced by a generated PNGs.


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

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



[GitHub] [airflow] kaxil commented on pull request #10380: Updated Documentation for the CI with mermaid sequence diagrams

Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #10380:
URL: https://github.com/apache/airflow/pull/10380#issuecomment-679226000


   Are the package.json and the files for building mermaid diagrams in some other PR? I remember seeing 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.

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



[GitHub] [airflow] kaxil commented on pull request #10380: Updated Documentation for the CI with mermaid sequence diagrams

Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #10380:
URL: https://github.com/apache/airflow/pull/10380#issuecomment-679215039


   Yes, I feel we could reduce the code to maintain by using something other than Mermaid, this is where I think draw.io might  be helpful 


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

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



[GitHub] [airflow] kaxil commented on pull request #10380: Updated Documentation for the CI with mermaid sequence diagrams

Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #10380:
URL: https://github.com/apache/airflow/pull/10380#issuecomment-679216493


   https://drawio-app.com/create-mermaid-diagrams-in-draw-io/ -> There is an integration that is already available


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

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



[GitHub] [airflow] potiuk commented on pull request #10380: Updated Documentation for the CI with mermaid sequence diagrams

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


   > https://drawio-app.com/create-mermaid-diagrams-in-draw-io/ -> There is an integration that is already available
   
   So you basically say you want to still keep the sources of mermaid but rather than doing `git commit` and have them automatically generated you want me to always go to draw,io, copy&paste the code, generate the image, download them, replace the images, commit (and not forget to copy back the sources if I changed them in draw,io?) 
   
   Because that's the alternative. Currently the only thing you need to do is to commit the sources and the images will be generated autinatically
   
   This is like forcing me to do those manual steps where simple (literally 20 lines of integration code that will never change) can do it for me. I do not really see the logic behind 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.

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



[GitHub] [airflow] potiuk commented on pull request #10380: Updated Documentation for the CI with mermaid sequence diagrams

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


   > Maintaining a separate node file just for mermaid makes 0 sense
   
   If package.json is the problem, then no worries.  I can remove the package.json and move it to the script generating mermaid (which will only be run when .mermaid files change). Woudl that 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.

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



[GitHub] [airflow] potiuk commented on pull request #10380: Updated Documentation for the CI with mermaid sequence diagrams

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


   Would love to merge the docs to close the task!


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

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



[GitHub] [airflow] potiuk commented on pull request #10380: Updated Documentation for the CI with mermaid sequence diagrams

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


   > Are the package.json and the files for building mermaid diagrams in some other PR? I remember seeing them
   
   Just looking at that. I think they are lost on rebase :)
   


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

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