You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "potiuk (via GitHub)" <gi...@apache.org> on 2024/12/22 19:19:29 UTC
[PR] Fix branch filter pattens for provider branches in ci workflow [airflow]
potiuk opened a new pull request, #45162:
URL: https://github.com/apache/airflow/pull/45162
I **hope** this time it will be ok:
https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#filter-pattern-cheat-sheet
<!--
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.
-->
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
<!-- Please keep an empty line above the dashes. -->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+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 a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix branch filter pattens for provider branches in ci workflow [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #45162:
URL: https://github.com/apache/airflow/pull/45162#discussion_r1895053319
##########
.github/workflows/ci.yml:
##########
@@ -21,9 +21,15 @@ on: # yamllint disable-line rule:truthy
schedule:
- cron: '28 1,7,13,19 * * *'
push:
- branches: ['v[0-9]+-[0-9]+-test', 'providers-*/v[0-9]+-[0-9]+]']
+ branches:
Review Comment:
Well. Just to have some interesting discussion on specs:
> Mhm, but reading the YAML specs in https://yaml.org/spec/1.2.2/#plain-style I think @shubhamraj-git is right. If the literals [ and ] arecontained then it is advised to quote it:
>> In addition, inside [flow collections](https://yaml.org/spec/1.2.2/#flow-collection-styles), or when used as [implicit keys](https://yaml.org/spec/1.2.2/#example-single-pair-explicit-entry), plain scalars must not contain the “[”, “]”, “{”, “}” and “,” characters.
* Yeah. "flow collections" was what it was before (and it was quoted) - but I just changed it to `block collections' :)
* Implicit keys - are different things.
So we are safe :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix branch filter pattens for provider branches in ci workflow [airflow]
Posted by "shubhamraj-git (via GitHub)" <gi...@apache.org>.
shubhamraj-git commented on code in PR #45162:
URL: https://github.com/apache/airflow/pull/45162#discussion_r1895050453
##########
.github/workflows/ci.yml:
##########
@@ -21,9 +21,15 @@ on: # yamllint disable-line rule:truthy
schedule:
- cron: '28 1,7,13,19 * * *'
push:
- branches: ['v[0-9]+-[0-9]+-test', 'providers-*/v[0-9]+-[0-9]+]']
+ branches:
Review Comment:
Ohh, thats good.
Thanks for the insights!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix branch filter pattens for provider branches in ci workflow [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #45162:
URL: https://github.com/apache/airflow/pull/45162#discussion_r1895053319
##########
.github/workflows/ci.yml:
##########
@@ -21,9 +21,15 @@ on: # yamllint disable-line rule:truthy
schedule:
- cron: '28 1,7,13,19 * * *'
push:
- branches: ['v[0-9]+-[0-9]+-test', 'providers-*/v[0-9]+-[0-9]+]']
+ branches:
Review Comment:
Well. Just to have some interesting discussion on specs:
> Mhm, but reading the YAML specs in https://yaml.org/spec/1.2.2/#plain-style I think @shubhamraj-git is right. If the literals [ and ] arecontained then it is advised to quote it:
> In addition, inside [flow collections](https://yaml.org/spec/1.2.2/#flow-collection-styles), or when used as [implicit keys](https://yaml.org/spec/1.2.2/#example-single-pair-explicit-entry), plain scalars must not contain the “[”, “]”, “{”, “}” and “,” characters.
* Yeah. "flow collections" was what it was before (and it was quoted) - but I just changed it to `block collections' :)
* Implicit keys - are different things.
So we are safe :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix branch filter pattens for provider branches in ci workflow [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #45162:
URL: https://github.com/apache/airflow/pull/45162#discussion_r1895051251
##########
.github/workflows/ci.yml:
##########
@@ -21,9 +21,15 @@ on: # yamllint disable-line rule:truthy
schedule:
- cron: '28 1,7,13,19 * * *'
push:
- branches: ['v[0-9]+-[0-9]+-test', 'providers-*/v[0-9]+-[0-9]+]']
+ branches:
Review Comment:
Mhm, but reading the YAML specs in https://yaml.org/spec/1.2.2/#plain-style I _think_ @shubhamraj-git is right. If the literals `[` and `]` arecontained then it is advised to quote it:
> In addition, inside [flow collections](https://yaml.org/spec/1.2.2/#flow-collection-styles), or when used as [implicit keys](https://yaml.org/spec/1.2.2/#example-single-pair-explicit-entry), plain scalars must not contain the “[”, “]”, “{”, “}” and “,” characters.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix branch filter pattens for provider branches in ci workflow [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #45162:
URL: https://github.com/apache/airflow/pull/45162#discussion_r1895045687
##########
.github/workflows/ci.yml:
##########
@@ -21,9 +21,15 @@ on: # yamllint disable-line rule:truthy
schedule:
- cron: '28 1,7,13,19 * * *'
push:
- branches: ['v[0-9]+-[0-9]+-test', 'providers-*/v[0-9]+-[0-9]+]']
+ branches:
Review Comment:
We had discussion about it with @jscheffl recently - I think it does not change safety, and while I favoured quoting, I see how a bit nicer it is when you can skip them.
@jscheffl -> consider I've learned something new :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix branch filter pattens for provider branches in ci workflow [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #45162:
URL: https://github.com/apache/airflow/pull/45162#discussion_r1895049708
##########
.github/workflows/ci.yml:
##########
@@ -21,9 +21,15 @@ on: # yamllint disable-line rule:truthy
schedule:
- cron: '28 1,7,13,19 * * *'
push:
- branches: ['v[0-9]+-[0-9]+-test', 'providers-*/v[0-9]+-[0-9]+]']
+ branches:
Review Comment:
YAML is made to be less boilerplate. No quotes needed except if you want to force bool/int to be string. I favor w/o quotes. But might be a matter of taste. No difference in security/safety. All YAML parsers work without. Else it is a bad YAML parser.
--> https://stackoverflow.com/questions/19109912/yaml-do-i-need-quotes-for-strings-in-yaml ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix branch filter pattens for provider branches in ci workflow [airflow]
Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on PR #45162:
URL: https://github.com/apache/airflow/pull/45162#issuecomment-2558583515
Tested here, yes hope it works this time :)
<img width="699" alt="image" src="https://github.com/user-attachments/assets/eca7cb14-99de-48c1-b44d-04c655cb9d87" />
https://regextester.github.io/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix branch filter pattens for provider branches in ci workflow [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #45162:
URL: https://github.com/apache/airflow/pull/45162
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix branch filter pattens for provider branches in ci workflow [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #45162:
URL: https://github.com/apache/airflow/pull/45162#issuecomment-2558584104
> Tested here, yes hope it works this time :)
>
The problem is ... it's not regexp. It's not glob either. It's something GitHub speciific https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#filter-pattern-cheat-sheet
For example they only support select ranges lile `[a-z]` ...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix branch filter pattens for provider branches in ci workflow [airflow]
Posted by "shubhamraj-git (via GitHub)" <gi...@apache.org>.
shubhamraj-git commented on code in PR #45162:
URL: https://github.com/apache/airflow/pull/45162#discussion_r1895040806
##########
.github/workflows/ci.yml:
##########
@@ -21,9 +21,15 @@ on: # yamllint disable-line rule:truthy
schedule:
- cron: '28 1,7,13,19 * * *'
push:
- branches: ['v[0-9]+-[0-9]+-test', 'providers-*/v[0-9]+-[0-9]+]']
+ branches:
Review Comment:
Should we include quotes for safe YAML parsing
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix branch filter pattens for provider branches in ci workflow [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #45162:
URL: https://github.com/apache/airflow/pull/45162#discussion_r1895049708
##########
.github/workflows/ci.yml:
##########
@@ -21,9 +21,15 @@ on: # yamllint disable-line rule:truthy
schedule:
- cron: '28 1,7,13,19 * * *'
push:
- branches: ['v[0-9]+-[0-9]+-test', 'providers-*/v[0-9]+-[0-9]+]']
+ branches:
Review Comment:
YAML is made to be less boilerplate. No quotes needed except if you want to force bool/int to be string. I favor w/o quotes. But might be a matter of taste. No difference in security/safety. All YAML parsers work without. Else it is a bad YAML parser.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix branch filter pattens for provider branches in ci workflow [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #45162:
URL: https://github.com/apache/airflow/pull/45162#discussion_r1895049708
##########
.github/workflows/ci.yml:
##########
@@ -21,9 +21,15 @@ on: # yamllint disable-line rule:truthy
schedule:
- cron: '28 1,7,13,19 * * *'
push:
- branches: ['v[0-9]+-[0-9]+-test', 'providers-*/v[0-9]+-[0-9]+]']
+ branches:
Review Comment:
YAML is made to be less boilerplate. No quotes needed except if you want to force bool/int to be string. I favor w/o quotes. But might be a matter of taste. No difference in security.
--
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