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 2021/01/07 21:06:49 UTC
[GitHub] [airflow] bbovenzi opened a new pull request #13551: Add JSON linter to Variable/DAG Trigger UIs
bbovenzi opened a new pull request #13551:
URL: https://github.com/apache/airflow/pull/13551
Added JSON linting the text input for add/edit a Variable and for the Trigger DAG config. Let's a user see any errors with their JSON input before submitting. This uses 2 external npm packages: `codemirror` and `jshint`.
closes: #12147
Some examples views below:
<img width="518" alt="Screen Shot 2021-01-07 at 1 47 38 PM" src="https://user-images.githubusercontent.com/4600967/103944617-8f86ff80-50f9-11eb-88ca-8dc81f23a054.png">
<img width="567" alt="Screen Shot 2021-01-07 at 1 48 16 PM" src="https://user-images.githubusercontent.com/4600967/103944619-8f86ff80-50f9-11eb-9e3f-dff876202eb4.png">
<img width="958" alt="Screen Shot 2021-01-07 at 1 47 26 PM" src="https://user-images.githubusercontent.com/4600967/103944615-8e55d280-50f9-11eb-8be4-916187b85182.png">
<img width="433" alt="Screen Shot 2021-01-07 at 3 05 04 PM" src="https://user-images.githubusercontent.com/4600967/103944739-bf360780-50f9-11eb-9a25-01e4f991a08c.png">
----------------------------------------------------------------
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 #13551: Add JSON linter to DAG Trigger UI
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #13551:
URL: https://github.com/apache/airflow/pull/13551#issuecomment-758113029
@bbovenzi - I am afraid you need to rebase (or `git commit --amend`) and `git push --force-with-lease`
----------------------------------------------------------------
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] ryanahamilton merged pull request #13551: Add JSON linter to DAG Trigger UI
Posted by GitBox <gi...@apache.org>.
ryanahamilton merged pull request #13551:
URL: https://github.com/apache/airflow/pull/13551
----------------------------------------------------------------
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] ryanahamilton commented on pull request #13551: Add JSON linter to DAG Trigger UI
Posted by GitBox <gi...@apache.org>.
ryanahamilton commented on pull request #13551:
URL: https://github.com/apache/airflow/pull/13551#issuecomment-760455927
CI failure appears unrelated.
----------------------------------------------------------------
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] github-actions[bot] commented on pull request #13551: Add JSON linter to DAG Trigger UI
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13551:
URL: https://github.com/apache/airflow/pull/13551#issuecomment-757014519
[The Workflow run](https://github.com/apache/airflow/actions/runs/472552212) is cancelling this PR. Building images for the PR has failed. Follow the the workflow link to check the reason.
----------------------------------------------------------------
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] bbovenzi commented on pull request #13551: Add JSON linter to Variable/DAG Trigger UIs
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on pull request #13551:
URL: https://github.com/apache/airflow/pull/13551#issuecomment-756941967
Since Variables can be JSON or plain text, I removed the Add/Edit Variable JSON linter for now.
Closes #11054 (Trigger DAG config can be more than just key/value pairs, so a JSON linter may be more useful)
----------------------------------------------------------------
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] bbovenzi edited a comment on pull request #13551: Add JSON linter to Variable/DAG Trigger UIs
Posted by GitBox <gi...@apache.org>.
bbovenzi edited a comment on pull request #13551:
URL: https://github.com/apache/airflow/pull/13551#issuecomment-756941967
Since Variables can be JSON or plain text, I removed the Add/Edit Variable JSON linter for now.
Also closes #11054 (Trigger DAG config can be more than just key/value pairs, so a JSON linter may be more useful)
----------------------------------------------------------------
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] github-actions[bot] commented on pull request #13551: Add JSON linter to DAG Trigger UI
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13551:
URL: https://github.com/apache/airflow/pull/13551#issuecomment-758310119
[The Workflow run](https://github.com/apache/airflow/actions/runs/478260169) is cancelling this PR. Building images for the PR has failed. Follow the the workflow link to check the reason.
----------------------------------------------------------------
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] boring-cyborg[bot] commented on pull request #13551: Add JSON linter to Variable/DAG Trigger UIs
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on pull request #13551:
URL: https://github.com/apache/airflow/pull/13551#issuecomment-756384460
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst)
Here are some useful points:
- Pay attention to the quality of your code (flake8, pylint and type annotations). Our [pre-commits]( https://github.com/apache/airflow/blob/master/STATIC_CODE_CHECKS.rst#prerequisites-for-pre-commit-hooks) will help you with that.
- In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/master/docs/apache-airflow/howto/custom-operator.rst) Consider adding an example DAG that shows how users should use it.
- Consider using [Breeze environment](https://github.com/apache/airflow/blob/master/BREEZE.rst) for testing locally, itโs a heavy docker but it ships with a working Airflow and a lot of integrations.
- Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
- Please follow [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct) for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
- Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#coding-style-and-best-practices).
Apache Airflow is a community-driven project and together we are making it better ๐.
In case of doubts contact the developers at:
Mailing List: dev@airflow.apache.org
Slack: https://s.apache.org/airflow-slack
----------------------------------------------------------------
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] bbovenzi edited a comment on pull request #13551: Add JSON linter to Variable/DAG Trigger UIs
Posted by GitBox <gi...@apache.org>.
bbovenzi edited a comment on pull request #13551:
URL: https://github.com/apache/airflow/pull/13551#issuecomment-756941967
----------------------------------------------------------------
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 #13551: Add JSON linter to DAG Trigger UI
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #13551:
URL: https://github.com/apache/airflow/pull/13551#issuecomment-758112488
> Wonder what has happened to CI...
@ashb: Just in case it happens again so that you know. This is a random error of the Github Packages infrastructure.
1) The image was correctly build and failed the first time it was pushed:
https://github.com/apache/airflow/runs/1670775083?check_suite_focus=true#step:10:443
```
manifest invalid: Only schema version 2 is supported
WARNING:: Error 1 when pushing image on 1 try
docker push docker.pkg.github.com/apache/airflow/master-python3.6-ci:e3eaf5160cc4a694afb05debf3ef81bf56694230
````
^^ This is retry and it succeeded.
2) The image is present (in theory) so "wait for image succeds". Wait for image (to save bandwith and time) only checks if the manifest exists using the registry API:
https://github.com/apache/airflow/pull/13551/checks?check_run_id=1670693749#step:5:221
3) But when you try to pull it (you can try it even now):
```
docker pull docker.pkg.github.com/apache/airflow/master-python3.6-ci:472552156
Error response from daemon: mediaType in manifest should be 'application/vnd.docker.distribution.manifest.v2+json' not ''
```
The problem is that GitHub has not 100% unstable architecture for "Github Packages" - looks like sometimes randomly the proxy
is redirecting the requests wrongly and they get to a wrong backend (this manifest itself as v1 vs. v2 manifest version problem). This is one of the main reasons they introduced GitHub Container Registry in September and deprecated GitHub Packages : https://github.blog/2020-09-01-introducing-github-container-registry/ (without the sunset time yet).
Apparenly there are cases where such image gets corupted (and there is not much we can do).
When I opened (Oct 10) Personal ticket https://support.github.com/ticket/personal/0/865404 (so you won't see) but I also documented it (last point in https://github.com/apache/airflow/issues/11294) they fixed "unknown blob" apparently but now we have 'invalid manifest"
Another ticket - where I verified that we can continue with Packages without the possibility of deleting the packages - I asked if their system is capable of handling our many images without deleting: https://support.github.com/ticket/personal/0/861667 - GH support confirmed that this will be no problem, but recommended us to switch to Container Registry (where old image retention can be implemented).
I think switching to container registry is the only way to solve it.
I opened (7th of October) a JIRA ticket to infra so that they consider enabling Container Registry for the projects so that we can switch to it: https://issues.apache.org/jira/projects/INFRA/issues/INFRA-20959
This ticket was waiting for the INFRA-organized meeting with GitHub Account, where we were supposed to discuss it but the meeting got cancelled and we are waiting for the new date (I asked for it several times).
Anything unclear - let me know @ashb
----------------------------------------------------------------
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] boring-cyborg[bot] commented on pull request #13551: Add JSON linter to DAG Trigger UI
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on pull request #13551:
URL: https://github.com/apache/airflow/pull/13551#issuecomment-760456155
Awesome work, congrats on your first merged pull request!
----------------------------------------------------------------
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] github-actions[bot] commented on pull request #13551: Add JSON linter to DAG Trigger UI
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13551:
URL: https://github.com/apache/airflow/pull/13551#issuecomment-756984542
----------------------------------------------------------------
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] bbovenzi edited a comment on pull request #13551: Add JSON linter to DAG Trigger UI
Posted by GitBox <gi...@apache.org>.
bbovenzi edited a comment on pull request #13551:
URL: https://github.com/apache/airflow/pull/13551#issuecomment-756941967
Since Variables can be JSON or plain text, I removed the Add/Edit Variable JSON linter for 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 pull request #13551: Add JSON linter to DAG Trigger UI
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #13551:
URL: https://github.com/apache/airflow/pull/13551#issuecomment-758112488
> Wonder what has happened to CI...
@ashb: Just in case it happens again so that you know. This is a random error of the Github Packages infrastructure.
1) The image was correctly build and failed the first time it was pushed:
https://github.com/apache/airflow/runs/1670775083?check_suite_focus=true#step:10:443
```
manifest invalid: Only schema version 2 is supported
WARNING:: Error 1 when pushing image on 1 try
docker push docker.pkg.github.com/apache/airflow/master-python3.6-ci:e3eaf5160cc4a694afb05debf3ef81bf56694230
````
^^ This is retry and it succeeded.
2) The image is present (in theory) so "wait for image succeds". Wait for image (to save bandwith and time) only checks if the manifest exists using the registry API:
https://github.com/apache/airflow/pull/13551/checks?check_run_id=1670693749#step:5:221
3) But when you try to pull it (you can try it even now):
```
docker pull docker.pkg.github.com/apache/airflow/master-python3.6-ci:472552156
Error response from daemon: mediaType in manifest should be 'application/vnd.docker.distribution.manifest.v2+json' not ''
```
The problem is that GitHub has not 100% unstable architecture for "Github Packages" - looks like sometimes randomly the proxy
is redirecting the requests wrongly and they get to a wrong backend (this manifest itself as v1 vs. v2 manifest version problem). This is one of the main reasons they introduced GitHub Container Registry in September and deprecated GitHub Packages : https://github.blog/2020-09-01-introducing-github-container-registry/ (without the sunset time yet).
Apparenly there are cases where such image gets corupted (and there is not much we can do).
When I opened (Oct 10) Personal ticket https://support.github.com/ticket/personal/0/865404 (so you won't see) but I also documented it (last point in https://github.com/apache/airflow/issues/11294) they fixed "unknown blob" apparently but now we have 'invalid manifest"
Another ticket - where I verified that we can continue with Packages without the possibility of deleting the packages - I asked if their system is capable of handling our many images without deleting: https://support.github.com/ticket/personal/0/861667 - GH support confirmed that this will be no problem, but recommended us to switch to Container Registry (where old image retention can be implemented).
I think switching to container registry is the only way to solve it.
I opened (7th of October) a JIRA ticket to infra so that they consider enabling Container Registry for the projects so that we can switch to it: https://issues.apache.org/jira/projects/INFRA/issues/INFRA-20959
This ticket was waiting for the INFRA-organized meeting with GitHub Account, where we were supposed to discuss it but the meeting got cancelled and we are waiting for the new date (I asked for it several times).
Anything unclear - let me know @ashb -> this ticket needs reb
----------------------------------------------------------------
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] bbovenzi commented on pull request #13551: Add JSON linter to Variable/DAG Trigger UIs
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on pull request #13551:
URL: https://github.com/apache/airflow/pull/13551#issuecomment-756941967
Since Variables can be JSON or plain text, I removed the Add/Edit Variable JSON linter for now.
Closes #11054 (Trigger DAG config can be more than just key/value pairs, so a JSON linter may be more useful)
----------------------------------------------------------------
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] github-actions[bot] commented on pull request #13551: Add JSON linter to DAG Trigger UI
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13551:
URL: https://github.com/apache/airflow/pull/13551#issuecomment-756984542
The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest master or amend the last commit of the PR, and push it with --force-with-lease.
----------------------------------------------------------------
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