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

Modify Pipe to return an RRef. #47829

Closed

Conversation

pritamdamania87
Copy link
Contributor

@pritamdamania87 pritamdamania87 commented Nov 12, 2020

Stack from ghstack:

As per proposal in #44827,
the API needs to return an RRef to support inter-host pipelining.

For now, we just return a local RRef and only support pipeline on a single
host. But having this change in the API upfront ensures we don't make any BC
breaking changes later.

Differential Revision: D24914022

As per proposal in #44827,
the API needs to return an RRef to support inter-host pipelining.

For now, we just return a local RRef and only support pipeline on a single
host. But having this change in the API upfront ensures we don't make any BC
breaking changes later.

Differential Revision: [D24914022](https://our.internmc.facebook.com/intern/diff/D24914022/)

[ghstack-poisoned]
@facebook-github-bot facebook-github-bot added cla signed oncall: distributed Add this issue/PR to distributed oncall triage queue labels Nov 12, 2020
pritamdamania87 pushed a commit that referenced this pull request Nov 12, 2020
As per proposal in #44827,
the API needs to return an RRef to support inter-host pipelining.

For now, we just return a local RRef and only support pipeline on a single
host. But having this change in the API upfront ensures we don't make any BC
breaking changes later.

Differential Revision: [D24914022](https://our.internmc.facebook.com/intern/diff/D24914022/)

ghstack-source-id: 116499310
Pull Request resolved: #47829
@dr-ci
Copy link

dr-ci bot commented Nov 12, 2020

💊 CI failures summary and remediations

As of commit d03229f (more details on the Dr. CI page):


💚 💚 Looks good so far! There are no failures yet. 💚 💚


This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

This comment has been revised 41 times.

import tempfile

@pytest.fixture
def setup_rpc(scope="session"):
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this function isn't used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's actually used as part of input to the test fixtures (ex: test_1to3).

Copy link
Member

@rohan-varma rohan-varma left a comment

Choose a reason for hiding this comment

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

LGTM, although there are lint errors.

As per proposal in #44827,
the API needs to return an RRef to support inter-host pipelining.

For now, we just return a local RRef and only support pipeline on a single
host. But having this change in the API upfront ensures we don't make any BC
breaking changes later.

Differential Revision: [D24914022](https://our.internmc.facebook.com/intern/diff/D24914022/)

[ghstack-poisoned]
pritamdamania87 pushed a commit that referenced this pull request Nov 25, 2020
Pull Request resolved: #47829

As per proposal in #44827,
the API needs to return an RRef to support inter-host pipelining.

For now, we just return a local RRef and only support pipeline on a single
host. But having this change in the API upfront ensures we don't make any BC
breaking changes later.
ghstack-source-id: 117361908

Differential Revision: [D24914022](https://our.internmc.facebook.com/intern/diff/D24914022/)
As per proposal in #44827,
the API needs to return an RRef to support inter-host pipelining.

For now, we just return a local RRef and only support pipeline on a single
host. But having this change in the API upfront ensures we don't make any BC
breaking changes later.

Differential Revision: [D24914022](https://our.internmc.facebook.com/intern/diff/D24914022/)

[ghstack-poisoned]
pritamdamania87 pushed a commit that referenced this pull request Nov 26, 2020
Pull Request resolved: #47829

As per proposal in #44827,
the API needs to return an RRef to support inter-host pipelining.

For now, we just return a local RRef and only support pipeline on a single
host. But having this change in the API upfront ensures we don't make any BC
breaking changes later.
ghstack-source-id: 117378144

Differential Revision: [D24914022](https://our.internmc.facebook.com/intern/diff/D24914022/)
As per proposal in #44827,
the API needs to return an RRef to support inter-host pipelining.

For now, we just return a local RRef and only support pipeline on a single
host. But having this change in the API upfront ensures we don't make any BC
breaking changes later.

Differential Revision: [D24914022](https://our.internmc.facebook.com/intern/diff/D24914022/)

[ghstack-poisoned]
pritamdamania87 pushed a commit that referenced this pull request Nov 30, 2020
Pull Request resolved: #47829

As per proposal in #44827,
the API needs to return an RRef to support inter-host pipelining.

For now, we just return a local RRef and only support pipeline on a single
host. But having this change in the API upfront ensures we don't make any BC
breaking changes later.
ghstack-source-id: 117494044

Differential Revision: [D24914022](https://our.internmc.facebook.com/intern/diff/D24914022/)
As per proposal in #44827,
the API needs to return an RRef to support inter-host pipelining.

For now, we just return a local RRef and only support pipeline on a single
host. But having this change in the API upfront ensures we don't make any BC
breaking changes later.

Differential Revision: [D24914022](https://our.internmc.facebook.com/intern/diff/D24914022/)

[ghstack-poisoned]
pritamdamania87 pushed a commit that referenced this pull request Dec 8, 2020
Pull Request resolved: #47829

As per proposal in #44827,
the API needs to return an RRef to support inter-host pipelining.

For now, we just return a local RRef and only support pipeline on a single
host. But having this change in the API upfront ensures we don't make any BC
breaking changes later.
ghstack-source-id: 118064022

Differential Revision: [D24914022](https://our.internmc.facebook.com/intern/diff/D24914022/)
As per proposal in #44827,
the API needs to return an RRef to support inter-host pipelining.

For now, we just return a local RRef and only support pipeline on a single
host. But having this change in the API upfront ensures we don't make any BC
breaking changes later.

Differential Revision: [D24914022](https://our.internmc.facebook.com/intern/diff/D24914022/)

[ghstack-poisoned]
pritamdamania87 pushed a commit that referenced this pull request Dec 10, 2020
Pull Request resolved: #47829

As per proposal in #44827,
the API needs to return an RRef to support inter-host pipelining.

For now, we just return a local RRef and only support pipeline on a single
host. But having this change in the API upfront ensures we don't make any BC
breaking changes later.
ghstack-source-id: 118252711

Differential Revision: [D24914022](https://our.internmc.facebook.com/intern/diff/D24914022/)
As per proposal in #44827,
the API needs to return an RRef to support inter-host pipelining.

For now, we just return a local RRef and only support pipeline on a single
host. But having this change in the API upfront ensures we don't make any BC
breaking changes later.

Differential Revision: [D24914022](https://our.internmc.facebook.com/intern/diff/D24914022/)

[ghstack-poisoned]
As per proposal in #44827,
the API needs to return an RRef to support inter-host pipelining.

For now, we just return a local RRef and only support pipeline on a single
host. But having this change in the API upfront ensures we don't make any BC
breaking changes later.

Differential Revision: [D24914022](https://our.internmc.facebook.com/intern/diff/D24914022/)

[ghstack-poisoned]
@facebook-github-bot facebook-github-bot deleted the gh/pritamdamania87/181/head branch December 15, 2020 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla signed Merged oncall: distributed Add this issue/PR to distributed oncall triage queue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants