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

Load module on remote workers from master (v2) #21695

Merged
merged 1 commit into from
May 16, 2017
Merged

Load module on remote workers from master (v2) #21695

merged 1 commit into from
May 16, 2017

Conversation

GregPlowman
Copy link
Contributor

@GregPlowman GregPlowman commented May 4, 2017 •
edited
Loading

Fix Issue #19960

Replace PR #21647

Is it OK to suggest some more labels?
backport pending 0.5 backport pending 0.6 bugfix

@GregPlowman GregPlowman mentioned this pull request May 4, 2017
Closed
@tkelman
Copy link
Contributor

tkelman commented May 4, 2017

Commit messages should really be more descriptive

@kshyatt kshyatt added parallelism Parallel or distributed computation modules labels May 4, 2017
@GregPlowman
Copy link
Contributor Author

Can this fix be backported to v0.5? (as per request for superseded PR)

@GregPlowman
Copy link
Contributor Author

I think this should be good to go now.

@StefanKarpinski
Copy link
Sponsor Member

I think this should be eligible for backporting. Unfortunately, 0.5.2 was just released, but we can do a 0.5.3 release in a month or so.

@JeffBezanson
Copy link
Sponsor Member

Needed on 0.6 as well I think?

@GregPlowman
Copy link
Contributor Author

Yes sorry, this is needed on 0.5, 0.6 and 0.7.
From my end, it's ready to merge into master.

@GregPlowman GregPlowman mentioned this pull request May 15, 2017
Merged
2 tasks
vchuravy
vchuravy approved these changes May 16, 2017
View reviewed changes
Copy link
Member

@vchuravy vchuravy left a comment

Choose a reason for hiding this comment

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

Looks good to me. Please squash the commits so that it is bisect friendly.

@vchuravy vchuravy requested a review from vtjnash May 16, 2017 01:07
@GregPlowman
Copy link
Contributor Author

GregPlowman commented May 16, 2017 •
edited
Loading

Ha! Looks like my squash worked!

@vchuravy vchuravy merged commit 448acca into JuliaLang:master May 16, 2017
@GregPlowman GregPlowman deleted the glp/remote_loading branch May 16, 2017 23:58
tkelman pushed a commit that referenced this pull request May 17, 2017
@GregPlowman @tkelman
Issue 19960 - loading module on remote workers
9564404 
(cherry picked from commit 10c0746)
ref #21695
@iamed2
Copy link
Contributor

iamed2 commented Jun 15, 2017

@tkelman Looks like this was missed for v0.5.3. Will we get this in a v0.5.4?

@GregPlowman
Copy link
Contributor Author

I think this should be eligible for backporting. Unfortunately, 0.5.2 was just released, but we can do a 0.5.3 release in a month or so.

Is there any update on when 0.5.3 might be released?

This was referenced Jun 27, 2017
Closed
Closed
@GregPlowman
Copy link
Contributor Author

Any update on backport to v0.5?

@tkelman
Copy link
Contributor

tkelman commented Aug 30, 2017

How critical is having this on 0.5? Most of the ecosystem is moving to 0.6-only now, I wasn't thinking another 0.5.x point release would be necessary.

@GregPlowman
Copy link
Contributor Author

GregPlowman commented Aug 30, 2017 •
edited
Loading

How critical is having this on 0.5? Most of the ecosystem is moving to 0.6-only now, I wasn't thinking another 0.5.x point release would be necessary.

Well I guess nothing is really critical.

For my main project, I'm still using v0.4.5, which is the last version where remote code loading worked for me. I wanted to upgrade to v0.5 as an intermediate step, before upgrading to v0.6. I suppose I can upgrade to v0.6 directly, but was holding out in anticipation of a 0.5.x release that includes the fix for this issue.

So I guess I can't really justify asking for another 0.5.x release just for this issue.
However, if for other reasons, there is going to be another release, then please backport this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vchuravy vchuravy vchuravy approved these changes

@vtjnash vtjnash Awaiting requested review from vtjnash

Assignees
No one assigned
Labels
modules parallelism Parallel or distributed computation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@GregPlowman @tkelman @StefanKarpinski @JeffBezanson @iamed2 @vchuravy @kshyatt