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

[AIRFLOW-4472] Use json.dumps/loads for templating lineage data #5253

Merged
merged 1 commit into from
May 7, 2019

Conversation

bolkedebruin
Copy link
Contributor

Make sure you have checked all steps below.

Jira

  • My PR addresses the following Airflow Jira issues and references them in the PR title. For example, "[AIRFLOW-XXX] My Airflow PR"
    • https://issues.apache.org/jira/browse/AIRFLOW-4472
    • In case you are fixing a typo in the documentation you can prepend your commit with [AIRFLOW-XXX], code changes always need a Jira issue.
    • In case you are proposing a fundamental code change, you need to create an Airflow Improvement Proposal (AIP).
    • In case you are adding a dependency, check if the license complies with the ASF 3rd Party License Policy.

Description

  • Here are some details about my PR, including screenshots of any UI changes:

jinja2 cannot use dict/lists as templates hence converting
it to json solves this while keeping complexity down.

Tests

  • My PR adds the following unit tests OR does not need testing for this extremely good reason:

Is covered.

Commits

  • My commits all reference Jira issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":
    1. Subject is separated from body by a blank line
    2. Subject is limited to 50 characters (not including Jira issue reference)
    3. Subject does not end with a period
    4. Subject uses the imperative mood ("add", not "adding")
    5. Body wraps at 72 characters
    6. Body explains "what" and "why", not "how"

Documentation

  • In case of new functionality, my PR adds documentation that describes how to use it.
    • All the public functions and the classes in the PR contain docstrings that explain what it does
    • If you implement backwards incompatible changes, please leave a note in the Updating.md so we can assign it to a appropriate release

Code Quality

  • Passes flake8

@bolkedebruin
Copy link
Contributor Author

cc: @feng-tao

This is a pre-requisite for the Papermill operator.

jinja2 cannot use dict/lists as templates hence converting
it to json solves this while keeping complexity down.
@bolkedebruin bolkedebruin merged commit a6daeb5 into apache:master May 7, 2019
JCoder01 pushed a commit to JCoder01/airflow that referenced this pull request May 14, 2019
…he#5253)

jinja2 cannot use dict/lists as templates hence converting
it to json solves this while keeping complexity down.
andriisoldatenko pushed a commit to andriisoldatenko/airflow that referenced this pull request Jul 26, 2019
…he#5253)

jinja2 cannot use dict/lists as templates hence converting
it to json solves this while keeping complexity down.
wmorris75 pushed a commit to modmed/incubator-airflow that referenced this pull request Jul 29, 2019
…he#5253)

jinja2 cannot use dict/lists as templates hence converting
it to json solves this while keeping complexity down.
@ssYkse
Copy link

ssYkse commented May 11, 2020

Hi!
This work, however for some reason is not part of the airflow 1.10.10 release (and earlier).
As it is merged into master, I assume it was lost somewhere in other merges.

@sankroh
Copy link

sankroh commented May 20, 2020

Can you guys release this please?

@Minutis
Copy link
Contributor

Minutis commented Jun 16, 2020

Would like to see this one released too.

@potiuk
Copy link
Member

potiuk commented Jun 16, 2020

I believe it was never meant to be released @kaxil @ashb @bolkedebruin? I am not 100% sure as it has no tests, but I think this one gets rather deeply into the operator/context behaviour and it might have some undesireable effects (on the other hand maybe it's better to cherry-pick it now because otherwise people might hit similar problems when migrating to 2.0)?

@bolkedebruin
Copy link
Contributor Author

It's meant to be updated significantly. It's about 70% there and I need some time to finish it.

@dimberman dimberman added this to the Airflow 1.10.11 milestone Jun 18, 2020
dimberman pushed a commit that referenced this pull request Jun 18, 2020
jinja2 cannot use dict/lists as templates hence converting
it to json solves this while keeping complexity down.

(cherry picked from commit a6daeb5)
kaxil pushed a commit that referenced this pull request Jun 19, 2020
jinja2 cannot use dict/lists as templates hence converting
it to json solves this while keeping complexity down.

(cherry picked from commit a6daeb5)
potiuk pushed a commit that referenced this pull request Jun 20, 2020
jinja2 cannot use dict/lists as templates hence converting
it to json solves this while keeping complexity down.

(cherry picked from commit a6daeb5)
potiuk pushed a commit that referenced this pull request Jun 29, 2020
jinja2 cannot use dict/lists as templates hence converting
it to json solves this while keeping complexity down.

(cherry picked from commit a6daeb5)
kaxil pushed a commit that referenced this pull request Jul 1, 2020
jinja2 cannot use dict/lists as templates hence converting
it to json solves this while keeping complexity down.

(cherry picked from commit a6daeb5)
kaxil pushed a commit to astronomer/airflow that referenced this pull request Aug 3, 2020
…he#5253)

jinja2 cannot use dict/lists as templates hence converting
it to json solves this while keeping complexity down.

(cherry picked from commit a6daeb5)
(cherry picked from commit 4b808ed)
cfei18 pushed a commit to cfei18/incubator-airflow that referenced this pull request Mar 5, 2021
jinja2 cannot use dict/lists as templates hence converting
it to json solves this while keeping complexity down.

(cherry picked from commit a6daeb5)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants