Skip to content

Address Github issue #11532 by translating legacy parameters for direct launches #11921

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

Closed
wants to merge 0 commits into from

Conversation

qkoziol
Copy link
Contributor

@qkoziol qkoziol commented Sep 9, 2023

Borrow code from the OMPI schizo module in PRRTE that translates legacy MCA parameters when an application is direct launched (PRRTE will translate legacy parameters when natively launched).

(cherry picked from commit 5d236e9)

@github-actions github-actions bot added this to the v5.0.0 milestone Sep 9, 2023
@qkoziol
Copy link
Contributor Author

qkoziol commented Sep 9, 2023

Updated my v5.0.x branch correctly and re-opened this PR.

@jsquyres
Copy link
Member

Just like #11916, CI is still failing.

Is there a reason you didn't just fix #11916? It is preferable to just fix existing PRs rather than close them and re-open new PRs (which effectively loses context and history).

@awlauria
Copy link
Contributor

my guess is a submodule update is needed for v5

@rhc54
Copy link
Contributor

rhc54 commented Sep 11, 2023

Yeah, this is failing because it is looking at old PMIx code. Commit #11920 and then rebase this and it should be okay.

@qkoziol
Copy link
Contributor Author

qkoziol commented Sep 11, 2023

Looks like I don't have #11920 underneath this in my 5.0.x branch yet. I'll pull it in and rebase, then force push this.

@awlauria
Copy link
Contributor

You shouldn't need to rebase.

bot:retest

@awlauria
Copy link
Contributor

@qkoziol the PR to v5 was merged, let's give this another ci run before closing/force pushing.

@awlauria
Copy link
Contributor

and too late..

@awlauria
Copy link
Contributor

Please do another force push to resolve the conflicts.

@jsquyres
Copy link
Member

@qkoziol Please stop closing PR's and opening new PR's. Please just fix your original PR.

@qkoziol
Copy link
Contributor Author

qkoziol commented Sep 11, 2023

Looks like "syncing my branch" in Github is what keeps closing the PR @jsquyres

@jsquyres
Copy link
Member

Looks like "syncing my branch" in Github is what keeps closing the PR @jsquyres

Hahah! You anticipated my comment. 😄

@jsquyres
Copy link
Member

@qkoziol I don't know what you're doing, but if you bring the branch on this PR up to the HEAD of the target branch of this PR (i.e., v5.0.x), I think Github may close your PR because it determines that there's no difference between your branch and the target branch...?

@qkoziol
Copy link
Contributor Author

qkoziol commented Sep 11, 2023

@qkoziol I don't know what you're doing, but if you bring the branch on this PR up to the HEAD of the target branch of this PR (i.e., v5.0.x), I think Github may close your PR because it determines that there's no difference between your branch and the target branch...?

Not certain ¯\(ツ)

I'm just going to stay away from the "sync branch" option, when the branch has an open PR...

@qkoziol
Copy link
Contributor Author

qkoziol commented Sep 11, 2023

FYI, new PR: #11923

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants