-
Notifications
You must be signed in to change notification settings - Fork 14.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added ds_format_locale method in macros which allows localizing datetime formatting using Babel #40746
Conversation
… allow locale (e.g. language) specific formatting of datetimes.
Just be aware that you will only be able to use for Airflow 2.10+ - I think it would be nice to add |
Also maybe worth to add a compat version to |
That would be the first thing added. |
Thank you Jarek for the tips will do that. |
What do I have to put there, as the macros is part of Airflow itself and is not in the providers? Docstring is updated as you suggested. |
I don’t like how this modifies the locale globally (albeit in a context manager). This tend to lead to bugs in concurrent situations. Is it possible to use Babel for this? We already have the package as a trasitive dependency (for various web packages). |
I've tried that. The problem with that is that it uses another formatting syntax, which made unit tests fail and which would be confusing and a breaking chang of that method. |
Sign. Let me dig around for alternatives… |
Apparently there’s not, Python’s API is just a very thin wrapper around the C implementation, and the C API can only use the current locale. Very bad but what can you expect from something designed 40 years ago. I guess a context manager is the best we can do for now—if bad things happen down the road we’ll add a lock to work around it. |
This also works, and that's converting the python datetime format to babel:
|
There are likely edge cases though (I didn’t check but it’s very likely). Let’s not try too hard on this. |
Yes but if you are worried of potential side-effects even when using a context manager, and I wasn't aware of this as I'm not that long in Pyhton, then I would opt for the later solution. And yes maybe there will be edge cases, BUT, at least it will be safe AND it's a new funcitonality, which means that if you don't pass the locale as argument, you're still using the original solution. If something would fail in combination wiht the locale, we can always fix it. All test cases are good with above solution, I would personally prefer this one over a hackish solution with context manager, wasn't happy about it in the first place so it's a good thing you pointed this out! ;) |
Or we add a new macro method ds_format_babel or ds_format_locale and specify there you have to use the babel format? |
I think adding a new macro based on Babel is a good idea! Not sure about the name but I like it otherwise. |
…o localize the datatime formatting using Babel
…ing of ds_format_locale
…ime formatting using Babel (apache#40746) --------- Co-authored-by: David Blain <[email protected]>
Added optional locale parameter to the ds_format method in macros to be able to specify a locale so datetimes can be formatted with the corresponding language.
For example following:
This can be very handy in jinja expressions where you need for example specifiy different formatting for multiple languages.
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
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.