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

[Core feature] Revisit enabling FlyteDeck by default for Python tasks. #5095

Open
2 tasks done
thomasjpfan opened this issue Mar 22, 2024 · 3 comments
Open
2 tasks done
Assignees
Labels
backlogged For internal use. Reserved for contributor team workflow. enhancement New feature or request flytekit FlyteKit Python related issue

Comments

@thomasjpfan
Copy link
Member

thomasjpfan commented Mar 22, 2024

Motivation: Why do you think this is important?

With flyteorg/flytekit#2248 and flyteorg/flytekit#2264, FlyteDecks will show the source code and python dependencies for Python tasks. This makes FlyteDecks universally useful for Python tasks.

Goal: What should the final outcome look like, ideally?

At least for Python tasks, it'll be great if enable_deck is True is the default.

Describe alternatives you've considered

The alternative is to keep the current default. Historically, FlyteDecks were turned off because there was a large overhead for rending large dataframes in the Inputs or Outputs.

We can mitigate this issue by trimming the source code being rendered and not show the inputs or outputs by default. I do not think there is a large use case for partially rendering inputs and outputs in the FlyteDeck.

An extension to this proposal is to add a new default_decks parameter to task:

@task(decks=["source_code", "dependencies"])

If a user wants to include the "input" deck, then can write: decks=["source_code", "dependencies", "input"]

Are you sure this issue hasn't been raised already?

  • Yes

Have you read the Code of Conduct?

  • Yes
@thomasjpfan thomasjpfan added enhancement New feature or request untriaged This issues has not yet been looked at by the Maintainers flytekit FlyteKit Python related issue labels Mar 22, 2024
Copy link

dosubot bot commented Mar 22, 2024

The proposal to enable FlyteDeck by default for Python tasks in Flytekit is an interesting one. As you've mentioned, the changes made in flytekit/pull/2248 and flytekit/pull/2264 allow FlyteDecks to show the source code and python dependencies for Python tasks. This could indeed enhance the user experience by providing more visibility into the tasks' data.

However, as you've pointed out, there are potential performance implications to consider. The generation of HTML reports for task inputs and outputs could introduce overhead, especially for tasks with large or complex outputs. This is something that needs to be carefully evaluated.

The alternative solution of keeping the current default due to the historical reason of FlyteDecks being turned off because of the large overhead for rendering large dataframes in the Inputs or Outputs is also a valid point. The proposed mitigation for this issue to trim the source code being rendered and not show the inputs or outputs by default could be a good compromise.

The suggestion to add a new default_decks parameter to task is a good one. This would give users more control over what they want to include in the decks. The example you provided:

@task(decks=["source_code", "dependencies"])

is a good illustration of how this could work. If a user wants to include the "input" deck, then they can write: decks=["source_code", "dependencies", "input"].

In conclusion, the proposal to enable FlyteDeck by default for Python tasks in Flytekit, with the addition of a new default_decks parameter to task, could be a good way forward. However, the potential performance implications need to be carefully evaluated and mitigated. It would be beneficial to gather more feedback from the community on this proposal.

Sources

About Dosu

This response is meant to be useful and save you time. It is not meant to be a precise solution, but rather a starting point for your own research.

Help me be more useful! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

If you want to continue the conversation, start your reply with @dosu-bot.

@thomasjpfan thomasjpfan added the backlogged For internal use. Reserved for contributor team workflow. label Mar 22, 2024
@pingsutw
Copy link
Member

+1 to enable it by default

@pingsutw pingsutw removed the untriaged This issues has not yet been looked at by the Maintainers label Mar 28, 2024
@novahow
Copy link
Contributor

novahow commented Mar 30, 2024

#take

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlogged For internal use. Reserved for contributor team workflow. enhancement New feature or request flytekit FlyteKit Python related issue
Projects
None yet
Development

No branches or pull requests

3 participants