Skip to content

Conversation

@thomasjpfan
Copy link
Contributor

@thomasjpfan thomasjpfan commented May 14, 2024

Tracking issue

Follow up to #2360

Why are the changes needed?

In #2360, we removed the cookiecutter dependency, which also removed jinja2. We require jinja2 to construct Flyte Decks.

What changes were proposed in this pull request?

This PR adds jinja2 as as a direct dependency.

How was this patch tested?

I built a Flytekit image without jinja2 installed and got this error:

{"asctime": "2024-05-14 16:16:05,571", "name": "flytekit", "levelname": "ERROR", "message": "Failed to write flyte deck html with error No module named 'jinja2'.", "taskName": null}

Once jinja2 is installed, the FlyteDeck button appears.

For reference the code is here:

def get_deck_template() -> "Template":
from jinja2 import Environment, FileSystemLoader, select_autoescape
root = os.path.dirname(os.path.abspath(__file__))
templates_dir = os.path.join(root, "html")
env = Environment(
loader=FileSystemLoader(templates_dir),
# 🔥 include autoescaping for security purposes
# sources:
# - https://jinja.palletsprojects.com/en/3.0.x/api/#autoescaping
# - https://stackoverflow.com/a/38642558/8474894 (see in comments)
# - https://stackoverflow.com/a/68826578/8474894
autoescape=select_autoescape(enabled_extensions=("html",)),
)
return env.get_template("template.html")

Signed-off-by: Thomas J. Fan <thomasjpfan@gmail.com>
@thomasjpfan thomasjpfan marked this pull request as ready for review May 14, 2024 18:42
Copy link
Contributor

@cosmicBboy cosmicBboy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm ok with this, will defer to @eapolinario or @wild-endeavor for second opinion

@cosmicBboy
Copy link
Contributor

the failing seems to be unrelated

@thomasjpfan
Copy link
Contributor Author

I'm planning to have a follow up PR to remove jinja2 again. I opened this PR to quickly fix the FlyteDeck issue.

@eapolinario
Copy link
Collaborator

dbt test failures are unrelated. Tracked in flyteorg/flyte#5351.

@eapolinario eapolinario merged commit 1ca3d91 into flyteorg:master May 15, 2024
austin362667 pushed a commit to austin362667/flytekit that referenced this pull request May 21, 2024
Signed-off-by: Thomas J. Fan <thomasjpfan@gmail.com>
austin362667 pushed a commit to austin362667/flytekit that referenced this pull request Jun 15, 2024
Signed-off-by: Thomas J. Fan <thomasjpfan@gmail.com>
austin362667 pushed a commit to austin362667/flytekit that referenced this pull request Jun 15, 2024
Signed-off-by: Thomas J. Fan <thomasjpfan@gmail.com>
fiedlerNr9 pushed a commit that referenced this pull request Jul 25, 2024
Signed-off-by: Thomas J. Fan <thomasjpfan@gmail.com>
Signed-off-by: Jan Fiedler <jan@union.ai>
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.

3 participants