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

fix: format commands output #3026

Merged
merged 1 commit into from
Jan 1, 2025
Merged

Conversation

vincent0426
Copy link
Contributor

@vincent0426 vincent0426 commented Dec 31, 2024

Tracking issue

Why are the changes needed?

better formatting

What changes were proposed in this pull request?

How was this patch tested?

Setup process

pyflyte run example.py --help

"""A basic Flyte project template that uses a Dockerfile"""

import typing
from flytekit import task, workflow


@task()
def say_hello(name: str) -> str:
    """A simple Flyte task to say "Hello".

    The @task decorator allows Flyte to use this function as a Flyte task,
    which is executed as an isolated, containerized unit of compute.
    """
    return f"Hello, {name}!"


@task()
def greeting_length(greeting: str) -> int:
    """A task the counts the length of a greeting."""
    return len(greeting)


@workflow
def wf(name: str = "world"):
    """Declare workflow called `wf`.

    The @workflow decorator defines an execution graph that is composed of
    tasks and potentially sub-workflows. In this simple example, the workflow
    is composed of just one task.

    There are a few important things to note about workflows:
    - Workflows are a domain-specific language (DSL) for creating execution
      graphs and therefore only support a subset of Python's behavior.
    - Tasks must be invoked with keyword arguments
    - The output variables of tasks are Promises, which are placeholders for
      values that are yet to be materialized, not the actual values.
    """
    greeting = say_hello(name=name)
    greeting_len = greeting_length(greeting=greeting)
    return greeting, greeting_len


if __name__ == "__main__":
    # Execute the workflow by invoking it like a function and passing in
    # the necessary parameters
    print(f"Running wf() {wf(name='passengers')}")

Screenshots

Before
Screenshot 2024-12-30 at 11 56 00 PM

After
Screenshot 2024-12-30 at 11 55 34 PM

Check all the applicable boxes

  • I updated the documentation accordingly.
  • All new and existing tests passed.
  • All commits are signed-off.

Related PRs

Docs link

Summary by Bito

Enhancement to Flyte CLI's visual presentation by implementing proper spacing in command documentation output. The update focuses on improving readability by adding appropriate spacing between command names and their descriptions, as evidenced through before/after screenshots.

Unit tests added: False

Estimated effort to review (1-5, lower is better): 1

Signed-off-by: Vincent <0426vincent@gmail.com>
Copy link

welcome bot commented Dec 31, 2024

Thank you for opening this pull request! 🙌

These tips will help get your PR across the finish line:

  • Most of the repos have a PR template; if not, fill it out to the best of your knowledge.
  • Sign off your commits (Reference: DCO Guide).

@flyte-bot
Copy link
Contributor

flyte-bot commented Dec 31, 2024

Code Review Agent Run #628a27

Actionable Suggestions - 0
Review Details
  • Files reviewed - 1 · Commit Range: 6adac09..6adac09
    • flytekit/clis/sdk_in_container/run.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

AI Code Review powered by Bito Logo

@flyte-bot
Copy link
Contributor

Changelist by Bito

This pull request implements the following key changes.

Key Change Files Impacted
Feature Improvement - Command Output Formatting Enhancement

run.py - Added space before command documentation for better readability

@kumare3 kumare3 merged commit c95cc63 into flyteorg:master Jan 1, 2025
102 checks passed
Copy link

welcome bot commented Jan 1, 2025

Congrats on merging your first pull request! 🎉

granthamtaylor added a commit that referenced this pull request Jan 6, 2025
* Store protos in local cache (#3022)

* Store proto obj instead of model Literal in local cache

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Remove unused file

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

---------

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
Co-authored-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Bump aiohttp from 3.9.5 to 3.10.11 (#3018)

Bumps [aiohttp](https://github.com/aio-libs/aiohttp) from 3.9.5 to 3.10.11.
- [Release notes](https://github.com/aio-libs/aiohttp/releases)
- [Changelog](https://github.com/aio-libs/aiohttp/blob/master/CHANGES.rst)
- [Commits](aio-libs/aiohttp@v3.9.5...v3.10.11)

---
updated-dependencies:
- dependency-name: aiohttp
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Fix bug in FlyteDirectory.listdir on local files (#2926)

* Fix issue in FlyteDirectory.listdir

Fixes flyteorg/flyte#6005

Signed-off-by: Pim de Haan <pim@cusp.ai>

* Added test

Signed-off-by: Pim de Haan <pim@cusp.ai>

* Run make lint

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

---------

Signed-off-by: Pim de Haan <pim@cusp.ai>
Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
Co-authored-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Fix unit tests in airflow plugin (#3024)

Signed-off-by: Kevin Su <pingsutw@apache.org>

* fix: Fix resource meta typos for async agent (#3023)

Signed-off-by: JiaWei Jiang <waynechuang97@gmail.com>

* fix: format commands output (#3026)

* Fix pydantic basemodel default input (#3013)

* Fix pydantic default input

Signed-off-by: Future-Outlier <eric901201@gmail.com>

* add pydantic integration test

Signed-off-by: Future-Outlier <eric901201@gmail.com>

* Use duck typing by Thomas's advice

Signed-off-by: Future-Outlier <eric901201@gmail.com>
Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>

* lint

Signed-off-by: Future-Outlier <eric901201@gmail.com>

---------

Signed-off-by: Future-Outlier <eric901201@gmail.com>
Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>

* [BUG] Open FlyteFile from remote path (#2991)

* fix: Open FlyteFile from remote path

Signed-off-by: JiaWei Jiang <waynechuang97@gmail.com>

* Add integration test

Signed-off-by: JiaWei Jiang <waynechuang97@gmail.com>

* refactor: Use ctx as param instead of recreation

Signed-off-by: JiaWei Jiang <waynechuang97@gmail.com>

* refactor: Clean test logic

1. Remove redundant prints
2. Use `mock.patch.dict` to setup `os.environ` for the current test fn
    * Avoid contaminating other tests running in the same process

Signed-off-by: JiaWei Jiang <waynechuang97@gmail.com>

* refactor: Setup local path and downloader in constructor

Signed-off-by: JiaWei Jiang <waynechuang97@gmail.com>

* refactor: Move SimpleFileTransfer to an utility file

Signed-off-by: JiaWei Jiang <waynechuang97@gmail.com>

* Remove redundant env var setup

Please refer to #3001

Signed-off-by: JiaWei Jiang <waynechuang97@gmail.com>

* test: Add another ff use case

Create ff in one task pod and read it in another task pod.

Signed-off-by: JiaWei Jiang <waynechuang97@gmail.com>

---------

Signed-off-by: JiaWei Jiang <waynechuang97@gmail.com>

* vllm inference plugin (#2967)

* vllm inference plugin

Signed-off-by: Daniel Sola <daniel.sola@union.ai>

* fixed default value

Signed-off-by: Daniel Sola <daniel.sola@union.ai>

---------

Signed-off-by: Daniel Sola <daniel.sola@union.ai>

* Add poetry to image spec (#3025)

* Add poetry to image spec

Signed-off-by: Thomas J. Fan <thomasjpfan@gmail.com>

* Add stricter check

Signed-off-by: Thomas J. Fan <thomasjpfan@gmail.com>

---------

Signed-off-by: Thomas J. Fan <thomasjpfan@gmail.com>

* [test] Add integration test for accessing sd sttr in dc (#2969)

* test: Add integration test for attr access of sd

Signed-off-by: JiaWei Jiang <waynechuang97@gmail.com>

* Correct file path

Signed-off-by: JiaWei Jiang <waynechuang97@gmail.com>

* test: Support interaction with minio s3 bucket

1. Upload a local parquet file to minio s3 bucket
2. Access StructuredDataset attr from a dataclass
3. Open StructuredDataset from a remote path

Signed-off-by: JiaWei Jiang <waynechuang97@gmail.com>

* Delete an unmerged integration test

Signed-off-by: JiaWei Jiang <waynechuang97@gmail.com>

* Try imagespec with commit sha of corresponding fix

Signed-off-by: JiaWei Jiang <waynechuang97@gmail.com>

* Remove redundant test

Signed-off-by: JiaWei Jiang <waynechuang97@gmail.com>

* Remove default_factory and create sd dc from input uri

Signed-off-by: JiaWei Jiang <waynechuang97@gmail.com>

* refactor: Clean test logic

1. Remove redundant prints
2. Use `mock.patch.dict` to setup `os.environ` for the current test fn
    * Avoid contaminating other tests running in the same process

Signed-off-by: JiaWei Jiang <waynechuang97@gmail.com>

* Remove redundant minio env var setup and add test comments

Signed-off-by: JiaWei Jiang <waynechuang97@gmail.com>

* Support uploading tmp pqt file

Signed-off-by: JiaWei Jiang <waynechuang97@gmail.com>

* Udpate deprecated module

Signed-off-by: JiaWei Jiang <waynechuang97@gmail.com>

* Remove redundant and unused imports

Signed-off-by: JiaWei Jiang <waynechuang97@gmail.com>

---------

Signed-off-by: JiaWei Jiang <waynechuang97@gmail.com>

---------

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: Pim de Haan <pim@cusp.ai>
Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: JiaWei Jiang <waynechuang97@gmail.com>
Signed-off-by: Future-Outlier <eric901201@gmail.com>
Signed-off-by: Daniel Sola <daniel.sola@union.ai>
Signed-off-by: Thomas J. Fan <thomasjpfan@gmail.com>
Co-authored-by: Eduardo Apolinario <653394+eapolinario@users.noreply.github.com>
Co-authored-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Pim de Haan <pimdehaan@gmail.com>
Co-authored-by: Kevin Su <pingsutw@apache.org>
Co-authored-by: 江家瑋 <36886416+JiangJiaWei1103@users.noreply.github.com>
Co-authored-by: V <0426vincent@gmail.com>
Co-authored-by: Han-Ru Chen (Future-Outlier) <eric901201@gmail.com>
Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
Co-authored-by: Daniel Sola <40698988+dansola@users.noreply.github.com>
shuyingliang pushed a commit to shuyingliang/flytekit that referenced this pull request Jan 11, 2025
Signed-off-by: Shuying Liang <shuying.liang@gmail.com>
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