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

Supports adding Mars extensions via setup entrypoints #2589

Merged
merged 3 commits into from
Dec 3, 2021
Merged

Supports adding Mars extensions via setup entrypoints #2589

merged 3 commits into from
Dec 3, 2021

Conversation

fyrestone
Copy link
Contributor

What do these changes do?

This PR defines an entrypoint for mars extensions. So, some packages can implement the entrypoint to make mars auto load the package as extension. Just the same as SDC is an extension of numba. Please refer to SDC for more details: https://github.com/IntelPython/sdc/blob/master/setup.py#L440

There are 4 load points:

  • LocalCluster
  • RayCluster
  • AbstractActorPool
  • new_session

The entrypoints will be loaded only once in a process.

Why not load entrypoints in mars.init?

  • Entrypoints may import mars for extension, this will cause recursive import error.

Related issue number

N/A

Check code requirements

  • tests added / passed (if needed)
  • Ensure all linting tests pass, see here for how to run them

wjsi
wjsi reviewed Dec 2, 2021
View reviewed changes
Copy link
Member

@wjsi wjsi left a comment

Choose a reason for hiding this comment

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

LGTM overall, I left a comment.

mars/core/entrypoints.py Outdated Show resolved Hide resolved
wjsi
wjsi approved these changes Dec 2, 2021
View reviewed changes
Copy link
Member

@wjsi wjsi left a comment

Choose a reason for hiding this comment

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

LGTM

@wjsi wjsi changed the title Mars extension support Supports adding Mars extension via setup entrypoints Dec 2, 2021
@wjsi wjsi changed the title Supports adding Mars extension via setup entrypoints Supports adding Mars extensions via setup entrypoints Dec 2, 2021
@wjsi wjsi added to be backported Indicate that the PR need to be backported to stable branch type: enhancement request type: install labels Dec 2, 2021
@wjsi wjsi added this to the v0.9.0a1 milestone Dec 2, 2021
qinxuye
qinxuye approved these changes Dec 3, 2021
View reviewed changes
Copy link
Collaborator

@qinxuye qinxuye left a comment

Choose a reason for hiding this comment

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

LGTM

@qinxuye qinxuye merged commit 4356fa1 into mars-project:master Dec 3, 2021
wjsi pushed a commit to wjsi/mars that referenced this pull request Dec 3, 2021
@fyrestone @wjsi
Supports adding Mars extensions via setup entrypoints (mars-project#2589
bfe6692 
)
@wjsi wjsi mentioned this pull request Dec 3, 2021
Merged
hekaisheng pushed a commit that referenced this pull request Dec 3, 2021
@wjsi @fyrestone
[BACKPORT] Supports adding Mars extensions via setup entrypoints (#2589…
f606687 
…) #2598

Co-authored-by: Liu Bao <fyrestone@outlook.com>
@hekaisheng hekaisheng added backported already PR has been backported and removed to be backported Indicate that the PR need to be backported to stable branch labels Dec 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wjsi wjsi wjsi approved these changes

@qinxuye qinxuye qinxuye approved these changes

@chaokunyang chaokunyang Awaiting requested review from chaokunyang

@hekaisheng hekaisheng Awaiting requested review from hekaisheng

Assignees
No one assigned
Labels
backported already PR has been backported type: enhancement request type: install
Projects
None yet
Milestone
v0.9.0a1
Development

Successfully merging this pull request may close these issues.
4 participants
@fyrestone @qinxuye @wjsi @hekaisheng