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/06/29 06:58:05 UTC
[GitHub] [airflow] NaveenGokavarapu19 opened a new pull request, #24724: Added Support in ts_nodash
NaveenGokavarapu19 opened a new pull request, #24724:
URL: https://github.com/apache/airflow/pull/24724
<!--
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 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/
-->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/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 a newsfragement file, named `{pr_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
[GitHub] [airflow] NaveenGokavarapu19 commented on a diff in pull request #24724: Added Support in ts_nodash
Posted by GitBox <gi...@apache.org>.
NaveenGokavarapu19 commented on code in PR #24724:
URL: https://github.com/apache/airflow/pull/24724#discussion_r910109685
##########
airflow/templates.py:
##########
@@ -56,9 +56,11 @@ def ts_filter(value):
return value.isoformat()
-def ts_nodash_filter(value):
- return value.strftime('%Y%m%dT%H%M%S')
-
+def ts_nodash_filter(*value):
+ if value:
+ return value.strftime('%Y%m%dT%H%M%S')
+ else:
+ return None
Review Comment:
I thought if we take the value as variable args it would work since if an empty list is passed or with values is it not the case. . And what is the meaning of falsy. Any docs or any blog to get more information about this.
--
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] NaveenGokavarapu19 commented on a diff in pull request #24724: Added Support in ts_nodash
Posted by GitBox <gi...@apache.org>.
NaveenGokavarapu19 commented on code in PR #24724:
URL: https://github.com/apache/airflow/pull/24724#discussion_r910109685
##########
airflow/templates.py:
##########
@@ -56,9 +56,11 @@ def ts_filter(value):
return value.isoformat()
-def ts_nodash_filter(value):
- return value.strftime('%Y%m%dT%H%M%S')
-
+def ts_nodash_filter(*value):
+ if value:
+ return value.strftime('%Y%m%dT%H%M%S')
+ else:
+ return None
Review Comment:
if thought if we take the value as variable args it would work since if an empty list is passed or with values . And what is the meaning of falsy. Any docs or any blog to get more information about this.
--
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] NaveenGokavarapu19 commented on a diff in pull request #24724: Added Support in ts_nodash
Posted by GitBox <gi...@apache.org>.
NaveenGokavarapu19 commented on code in PR #24724:
URL: https://github.com/apache/airflow/pull/24724#discussion_r910569237
##########
airflow/templates.py:
##########
@@ -56,9 +56,11 @@ def ts_filter(value):
return value.isoformat()
-def ts_nodash_filter(value):
- return value.strftime('%Y%m%dT%H%M%S')
-
+def ts_nodash_filter(*value):
+ if value:
+ return value.strftime('%Y%m%dT%H%M%S')
+ else:
+ return None
Review Comment:
Ok
--
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] uranusjr closed pull request #24724: Added Support in ts_nodash
Posted by GitBox <gi...@apache.org>.
uranusjr closed pull request #24724: Added Support in ts_nodash
URL: https://github.com/apache/airflow/pull/24724
--
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] uranusjr commented on a diff in pull request #24724: Added Support in ts_nodash
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #24724:
URL: https://github.com/apache/airflow/pull/24724#discussion_r909593111
##########
airflow/templates.py:
##########
@@ -56,9 +56,11 @@ def ts_filter(value):
return value.isoformat()
-def ts_nodash_filter(value):
- return value.strftime('%Y%m%dT%H%M%S')
-
+def ts_nodash_filter(*value):
+ if value:
+ return value.strftime('%Y%m%dT%H%M%S')
+ else:
+ return None
Review Comment:
This is wrong since
1. `datetime(0, 0, 0)` is falsy.
2. The `*` should not exist.
Something like this should be implemented instead
```python
def ts_nodash_filter(value):
if value is None:
return None
return value.strftime('%Y%m%dT%H%M%S')
```
Also please add the same logic to `ds_filter`, `ds_nodash_filter`, `ts_filter`, and `ts_nodash_with_tz_filter` as well.
--
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] uranusjr commented on a diff in pull request #24724: Added Support in ts_nodash
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #24724:
URL: https://github.com/apache/airflow/pull/24724#discussion_r910238535
##########
airflow/templates.py:
##########
@@ -56,9 +56,11 @@ def ts_filter(value):
return value.isoformat()
-def ts_nodash_filter(value):
- return value.strftime('%Y%m%dT%H%M%S')
-
+def ts_nodash_filter(*value):
+ if value:
+ return value.strftime('%Y%m%dT%H%M%S')
+ else:
+ return None
Review Comment:
Search “falsy python” in any search engine.
> I thought if we take the value as variable args it would work since if an empty list is passed or with values is it not the case
It is not, the filter is passed one single value, the datetime, and we are adding None to the possible input type. Please read up the documentation to understand how the filters are used. https://airflow.apache.org/docs/apache-airflow/stable/templates-ref.html#filters
--
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] uranusjr commented on pull request #24724: Added Support in ts_nodash
Posted by GitBox <gi...@apache.org>.
uranusjr commented on PR #24724:
URL: https://github.com/apache/airflow/pull/24724#issuecomment-1170807139
Please read https://github.com/apache/airflow/blob/main/STATIC_CODE_CHECKS.rst
--
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] uranusjr commented on pull request #24724: Added Support in ts_nodash
Posted by GitBox <gi...@apache.org>.
uranusjr commented on PR #24724:
URL: https://github.com/apache/airflow/pull/24724#issuecomment-1211618309
Superceded.
--
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] NaveenGokavarapu19 commented on pull request #24724: Added Support in ts_nodash
Posted by GitBox <gi...@apache.org>.
NaveenGokavarapu19 commented on PR #24724:
URL: https://github.com/apache/airflow/pull/24724#issuecomment-1170802650
i am unable to understand why static checks are failing is it related to indentation ??
--
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] uranusjr commented on a diff in pull request #24724: Added Support in ts_nodash
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #24724:
URL: https://github.com/apache/airflow/pull/24724#discussion_r910238535
##########
airflow/templates.py:
##########
@@ -56,9 +56,11 @@ def ts_filter(value):
return value.isoformat()
-def ts_nodash_filter(value):
- return value.strftime('%Y%m%dT%H%M%S')
-
+def ts_nodash_filter(*value):
+ if value:
+ return value.strftime('%Y%m%dT%H%M%S')
+ else:
+ return None
Review Comment:
Search “falsy python” in any search engine.
> I thought if we take the value as variable args it would work since if an empty list is passed or with values is it not the case
It is not, the filter is passed one single value, the datetime, and we are adding None to the possible input type.
--
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