Skip to content
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

Improved UI rendering of templated dicts and lists #17239

Closed
wants to merge 4 commits into from
Closed

Improved UI rendering of templated dicts and lists #17239

wants to merge 4 commits into from

Conversation

mehmax
Copy link
Contributor

@mehmax mehmax commented Jul 26, 2021

UI displayed rendered template dicts and lists as a json.dump.
This will not add any line breaks.
Especially when handling templated files (e.g. sql) line breaks will make the display more beautiful.
UI will now render each key+value from dicts and each index+value from lists seperately as own lines.

Example

Dictionaries

Old

grafik

New

grafik

Lists

Old

grafik

New

grafik

Nested dicts/lists

Old

grafik

New

grafik

Mixed

Old

grafik

New

grafik

It is now also possible to specify a dictionary or list path in template_field_renderers.
e.g.
{'configurations.sql' : 'sql' }
or
{'configurations[0] : 'sql'}

even nested dict/list paths are possible

{'configurations[0].sql : 'sql'}

closes: #17032


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, 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 UPDATING.md.

This will not add any line breaks.
Especially when handling templated files (e.g. sql) line breaks will make the display more beautiful.

UI will now render each key+value from dicts and each index+value from lists seperately as own lines.

e.g
configurations
{
	"key1" : "value1"
	"key2" : "value2"
}

will be shown as

configurations.key1
value1

configurations.key2
value2
@boring-cyborg boring-cyborg bot added the area:webserver Webserver related Issues label Jul 26, 2021
@boring-cyborg
Copy link

boring-cyborg bot commented Jul 26, 2021

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/main/CONTRIBUTING.rst)
Here are some useful points:

  • Pay attention to the quality of your code (flake8, mypy and type annotations). Our pre-commits 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 Consider adding an example DAG that shows how users should use it.
  • Consider using Breeze environment 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 for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
  • Be sure to read the Airflow Coding style.
    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

@mehmax mehmax changed the title UI displayed rendered template dicts and lists as a json.dump. Improved UI rendering of templated dicts and lists Jul 26, 2021
@mehmax mehmax marked this pull request as draft July 27, 2021 06:25
@mehmax
Copy link
Contributor Author

mehmax commented Jul 27, 2021

Changed this to a draft, because I noticed that this can be improve this a little, so that template_field_renderers defined in the operator will be applied as well.

@potiuk
Copy link
Member

potiuk commented Jul 27, 2021

Changed this to a draft, because I noticed that this can be improve this a little, so that template_field_renderers defined in the operator will be applied as well.

Looks good already :) @nathadfield ? And yeah - automated template_field_renderers application is perfect.

@ashb
Copy link
Member

ashb commented Jul 27, 2021

Rather than .0 can we have the python convention of [0] please.

@mehmax
Copy link
Contributor Author

mehmax commented Jul 30, 2021

Hi @nathadfield @potiuk I am quite new to contributing. Would you mind taking a look at this and give me some feedback? :)

My approach is a bit different to yours @nathadfield. I wanted to expand every dict / list by default. If list or dict item is listed in template_field_renderes it will applied as well.

Thank you guys!

@nathadfield
Copy link
Collaborator

@mehmax I am by no means a seasoned contributor either! I did try to accommodate the expanding and rendering of 'op_kwargs` in my PR #17321 which I don't know if you've looked at it.

We definitely have similar and overlapping interests here so I'd be keen to hear from others about what we've both done and how we can bring them together.

For example, I could scale back the scope of mine so that it only focuses on delivering dictionary path rendering to operators.

@mehmax
Copy link
Contributor Author

mehmax commented Jul 30, 2021

Rather than .0 can we have the python convention of [0] please.

done!

@potiuk
Copy link
Member

potiuk commented Aug 2, 2021

I guess we can close it as #17321 has been merged.

@potiuk potiuk closed this Aug 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:webserver Webserver related Issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improved SQL rendering within BigQueryInsertJobOperator
4 participants