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.

Sign up for GitHub

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

Jump to bottom

[dashboard] apply isort to dashboard dir #46483

Merged
merged 1 commit into from
Jul 8, 2024
Merged

[dashboard] apply isort to dashboard dir #46483

merged 1 commit into from
Jul 8, 2024

Conversation

aslonnie
Copy link
Collaborator

@aslonnie aslonnie commented Jul 8, 2024

sorting mostly imports

@aslonnie
[dashboard] apply isort to dashboard dir
600988e 
sorting mostly imports

Signed-off-by: Lonnie Liu <lonnie@anyscale.com>
@aslonnie aslonnie requested a review from alanwguo July 8, 2024 19:58
@aslonnie aslonnie mentioned this pull request Jul 8, 2024
Merged
@aslonnie aslonnie added the go add ONLY when ready to merge, run all tests label Jul 8, 2024
@jjyao
Copy link
Collaborator

jjyao commented Jul 8, 2024

We have check_import_order.py and we need to make sure

"""
This script ensures python files conform to ray's import ordering rules.
In particular, we make sure psutil and setproctitle is imported _after_
importing ray due to our bundling of the two libraries.

Usage:
$ python check_import_order.py SOURCE_DIR -s SKIP_DIR
some/file/path.py:23 import psutil without explicitly import ray before it.
"""

Just FYI

aslonnie
aslonnie commented Jul 8, 2024
View reviewed changes
dashboard/modules/job/job_supervisor.py
Comment on lines +25 to +26
import psutil

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@jjyao ... so this is fixed by isort?

dashboard/modules/job/tests/test_job_manager.py
from ray.tests.conftest import call_ray_start # noqa: F401
from ray.util.scheduling_strategies import NodeAffinitySchedulingStrategy # noqa: F401

import psutil
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

also here

dashboard/modules/reporter/reporter_agent.py
MemoryProfilingManager,
)

import psutil
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

and here

dashboard/modules/reporter/tests/test_reporter.py
from ray.dashboard.modules.reporter.reporter_agent import ReporterAgent
from ray.dashboard.tests.conftest import * # noqa
from ray.dashboard.utils import Bunch
from ray.core.generated.metrics_pb2 import Metric

import psutil
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

and here

dashboard/modules/metrics/metrics_head.py
)

import psutil
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

and here

@aslonnie aslonnie requested a review from rynewang July 8, 2024 20:27
alanwguo
alanwguo approved these changes Jul 8, 2024
View reviewed changes
Copy link
Contributor

@alanwguo alanwguo left a comment

Choose a reason for hiding this comment

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

Did some manual testing, things seem to continue to work.

@aslonnie
Copy link
Collaborator Author

aslonnie commented Jul 8, 2024

merging with @alanwguo 's blessings~

@aslonnie aslonnie merged commit bbcd01f into master Jul 8, 2024
5 checks passed
@aslonnie aslonnie deleted the lonnie-240708-dashsort branch July 8, 2024 23:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alanwguo alanwguo alanwguo approved these changes

@ericl ericl Awaiting requested review from ericl

@richardliaw richardliaw Awaiting requested review from richardliaw

@rkooo567 rkooo567 Awaiting requested review from rkooo567

@jjyao jjyao Awaiting requested review from jjyao

@rynewang rynewang Awaiting requested review from rynewang

Assignees
No one assigned
Labels
go add ONLY when ready to merge, run all tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@aslonnie @jjyao @alanwguo