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/20 21:42:57 UTC
[GitHub] [airflow] pierrejeambrun opened a new pull request, #25167: Add ts types generation to static checks
pierrejeambrun opened a new pull request, #25167:
URL: https://github.com/apache/airflow/pull/25167
Following https://github.com/apache/airflow/pull/25123, allowing us to auto-generate typescript types for the api, here is a pre-commit hook to ensure that these types stay in sync with the OpenAPI spec.
Modifying the `openapi/v1.yaml` will trigger the type generation. (then could eventually break ESlint hook when attempting again to commit, in case of incompatible changes)
--
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] Kinceo1 commented on pull request #25167: Add ts types generation to static checks
Posted by GitBox <gi...@apache.org>.
Kinceo1 commented on PR #25167:
URL: https://github.com/apache/airflow/pull/25167#issuecomment-1190780850
King
--
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] bbovenzi commented on pull request #25167: Generate typescript types from rest API docs
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on PR #25167:
URL: https://github.com/apache/airflow/pull/25167#issuecomment-1190052018
I merged my PR. Want to rebase and update the PR title and description?
--
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 #25167: Add ts types generation to static checks
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #25167:
URL: https://github.com/apache/airflow/pull/25167#issuecomment-1190776528
BTW. Is it possible @pierrejeambrun - you also remove all the node versions 18.6.0 in all node pre-commits and only update the version in the "top-level" default_language_version? I just realized that we have it by looking at those cached envs.
--
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 #25167: Add ts types generation to static checks
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #25167:
URL: https://github.com/apache/airflow/pull/25167#issuecomment-1190819880
> > Closed/reopened to rebuild as there was temp dockerhub failure.
>
> Thanks,
>
> I didn't know that trick, I'm used to amending and repushing 😂
That's what I do with my changes 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] pierrejeambrun commented on a diff in pull request #25167: Add ts types generation to static checks
Posted by GitBox <gi...@apache.org>.
pierrejeambrun commented on code in PR #25167:
URL: https://github.com/apache/airflow/pull/25167#discussion_r926077410
##########
.pre-commit-config.yaml:
##########
@@ -834,12 +834,12 @@ repos:
entry: ./scripts/ci/pre_commit/pre_commit_ui_lint.py
pass_filenames: false
additional_dependencies: ['yarn@1.22.19']
- - id: lint-javascript
+ - id: ts-compile-and-lint-javascript
name: ESLint against current UI JavaScript files
Review Comment:
I went for `TS types generation and ESLint against current UI 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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] pierrejeambrun commented on pull request #25167: Add "generate-api-types" pre-commit hook
Posted by GitBox <gi...@apache.org>.
pierrejeambrun commented on PR #25167:
URL: https://github.com/apache/airflow/pull/25167#issuecomment-1190746979
Following your suggestions, here is another option that merges the `generate-api-types` hook into `lint-javascript` (+ renaming reflecting this change).
Let me know what version you prefer :)
--
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 #25167: Add ts types generation to static checks
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #25167:
URL: https://github.com/apache/airflow/pull/25167#issuecomment-1190775008
I thin this one is better. In VAST majority of cases running 'generate-api-types' will produce the same file.
Unfortunately - unlike in Python, pre-commit does not reuse node environments, so the less of them, the better for time/cache restore and creation etc:
```
⌁ [jarek:~/.cache/pre-commit] % echo */*/lib/node_modules | xargs du -h -s
21M repocw333njc/node_env-18.6.0/lib/node_modules
48M repoltxx29sq/node_env-18.6.0/lib/node_modules
19M repowd9np_qb/node_env-18.6.0/lib/node_modules
20M repoy6z0rp4o/node_env-16.15.1/lib/node_modules
```
--
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 #25167: Add ts types generation to static checks
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #25167:
URL: https://github.com/apache/airflow/pull/25167#discussion_r926066877
##########
.pre-commit-config.yaml:
##########
@@ -834,12 +834,12 @@ repos:
entry: ./scripts/ci/pre_commit/pre_commit_ui_lint.py
pass_filenames: false
additional_dependencies: ['yarn@1.22.19']
- - id: lint-javascript
+ - id: ts-compile-and-lint-javascript
name: ESLint against current UI JavaScript files
Review Comment:
Let's change the description 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] pierrejeambrun commented on pull request #25167: Add "generate-api-types" pre-commit hook
Posted by GitBox <gi...@apache.org>.
pierrejeambrun commented on PR #25167:
URL: https://github.com/apache/airflow/pull/25167#issuecomment-1190712797
The `compile-www-assets` has a `manual` confined stage, should I extend this with commit ?
--
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] pierrejeambrun commented on pull request #25167: Add ts types generation to static checks
Posted by GitBox <gi...@apache.org>.
pierrejeambrun commented on PR #25167:
URL: https://github.com/apache/airflow/pull/25167#issuecomment-1190785720
Done, using the default node version and updated it to `18.6.0`
--
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] pierrejeambrun commented on pull request #25167: Add ts types generation to static checks
Posted by GitBox <gi...@apache.org>.
pierrejeambrun commented on PR #25167:
URL: https://github.com/apache/airflow/pull/25167#issuecomment-1190775810
Ok great thanks for the details :)
--
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 #25167: Add "generate-api-types" pre-commit hook
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #25167:
URL: https://github.com/apache/airflow/pull/25167#issuecomment-1190698605
One small comment - it woudl be - I think - much better to merge this one with `compile-www-assets` - then same node env will be used and single yarn install command would be needed
--
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] pierrejeambrun commented on a diff in pull request #25167: Add ts types generation to static checks
Posted by GitBox <gi...@apache.org>.
pierrejeambrun commented on code in PR #25167:
URL: https://github.com/apache/airflow/pull/25167#discussion_r926077410
##########
.pre-commit-config.yaml:
##########
@@ -834,12 +834,12 @@ repos:
entry: ./scripts/ci/pre_commit/pre_commit_ui_lint.py
pass_filenames: false
additional_dependencies: ['yarn@1.22.19']
- - id: lint-javascript
+ - id: ts-compile-and-lint-javascript
name: ESLint against current UI JavaScript files
Review Comment:
I went for `TS types generation and ESLint against current UI files` (remove the reference to Javascript as it is not really accurate anymore)
--
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 #25167: Add ts types generation to static checks
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #25167:
URL: https://github.com/apache/airflow/pull/25167#issuecomment-1190786939
Closed/reopened to rebuild as there was temp dockerhub failure.
--
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 #25167: Add ts types generation to static checks
Posted by GitBox <gi...@apache.org>.
potiuk merged PR #25167:
URL: https://github.com/apache/airflow/pull/25167
--
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] pierrejeambrun commented on a diff in pull request #25167: Add ts types generation to static checks
Posted by GitBox <gi...@apache.org>.
pierrejeambrun commented on code in PR #25167:
URL: https://github.com/apache/airflow/pull/25167#discussion_r926077410
##########
.pre-commit-config.yaml:
##########
@@ -834,12 +834,12 @@ repos:
entry: ./scripts/ci/pre_commit/pre_commit_ui_lint.py
pass_filenames: false
additional_dependencies: ['yarn@1.22.19']
- - id: lint-javascript
+ - id: ts-compile-and-lint-javascript
name: ESLint against current UI JavaScript files
Review Comment:
I went for `TS types generation and ESLint against current UI files` (that is bellow < 70char) I wanted to remove the reference to Javascript as it is not really accurate anymore
--
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] pierrejeambrun commented on pull request #25167: Add ts types generation to static checks
Posted by GitBox <gi...@apache.org>.
pierrejeambrun commented on PR #25167:
URL: https://github.com/apache/airflow/pull/25167#issuecomment-1190787432
> Closed/reopened to rebuild as there was temp dockerhub failure.
Thanks (didn't know that trick :p) I'm use to amending and repushing haha
--
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 closed pull request #25167: Add ts types generation to static checks
Posted by GitBox <gi...@apache.org>.
potiuk closed pull request #25167: Add ts types generation to static checks
URL: https://github.com/apache/airflow/pull/25167
--
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 #25167: Add ts types generation to static checks
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #25167:
URL: https://github.com/apache/airflow/pull/25167#discussion_r926073967
##########
.pre-commit-config.yaml:
##########
@@ -834,12 +834,12 @@ repos:
entry: ./scripts/ci/pre_commit/pre_commit_ui_lint.py
pass_filenames: false
additional_dependencies: ['yarn@1.22.19']
- - id: lint-javascript
+ - id: ts-compile-and-lint-javascript
name: ESLint against current UI JavaScript files
Review Comment:
`Generat and lint UI Javascript files` ?
##########
.pre-commit-config.yaml:
##########
@@ -834,12 +834,12 @@ repos:
entry: ./scripts/ci/pre_commit/pre_commit_ui_lint.py
pass_filenames: false
additional_dependencies: ['yarn@1.22.19']
- - id: lint-javascript
+ - id: ts-compile-and-lint-javascript
name: ESLint against current UI JavaScript files
Review Comment:
`Generate and lint UI Javascript 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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] pierrejeambrun commented on a diff in pull request #25167: Add ts types generation to static checks
Posted by GitBox <gi...@apache.org>.
pierrejeambrun commented on code in PR #25167:
URL: https://github.com/apache/airflow/pull/25167#discussion_r926071060
##########
.pre-commit-config.yaml:
##########
@@ -834,12 +834,12 @@ repos:
entry: ./scripts/ci/pre_commit/pre_commit_ui_lint.py
pass_filenames: false
additional_dependencies: ['yarn@1.22.19']
- - id: lint-javascript
+ - id: ts-compile-and-lint-javascript
name: ESLint against current UI JavaScript files
Review Comment:
Yes I didn't change it cause we are limited in length :(, trying to find something better
--
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 #25167: Add "generate-api-types" pre-commit hook
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #25167:
URL: https://github.com/apache/airflow/pull/25167#issuecomment-1190723759
On the other hand. isolation is better in this case rather than optimisation.
--
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 #25167: Add "generate-api-types" pre-commit hook
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #25167:
URL: https://github.com/apache/airflow/pull/25167#issuecomment-1190700662
Those are triggered by different source files - but I think overall it would be more efficient.
--
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] pierrejeambrun commented on pull request #25167: Add "generate-api-types" pre-commit hook
Posted by GitBox <gi...@apache.org>.
pierrejeambrun commented on PR #25167:
URL: https://github.com/apache/airflow/pull/25167#issuecomment-1190692946
The PR has been updated, we should be good to go!
--
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 #25167: Add "generate-api-types" pre-commit hook
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #25167:
URL: https://github.com/apache/airflow/pull/25167#issuecomment-1190722591
Right.... Forgot about it 🤦 But the "lint" one does not :). This what Is actually better "typescript compile lint" sems like 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