Skip to content

Conversation

tobiasdiez
Copy link
Contributor

The macos workflow is largely broken (e.g. various build timeouts and the second stage is never run since at least two months https://github.com/sagemath/sage/actions/runs/6332061110) and we only have 5 macos runners, so we reduce the impact of the macos workflow on other workflows (mostly the conda jobs) by running almost all its jobs in series instead of parallel (and the stage 1 is now limited to 2 runners).

📝 Checklist

  • The title is concise, informative, and self-explanatory.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation accordingly.

⌛ Dependencies

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 15, 2023

That the second stage is never run was fixed in #36711.

The "CI conda" workflows are piling up even when there's no "CI macOS" running -- as clearly explained in #36694.

For example, in the current run, there's actually nothing running because I made a typo in macos.yml. See https://github.com/sagemath/sage/actions/runs/6870325982

So I propose to close this PR.

@tobiasdiez
Copy link
Contributor Author

That the second stage is never run was fixed in #36711.

That nonetheless took almost two months to fix. My point is less that the macos are broken now, than that there apparently is no incentive from the community to fix issues with them in the first 10h after a release. So they can also wait 15-20h.

@mkoeppe
Copy link
Contributor

mkoeppe commented Dec 6, 2023

that there apparently is no incentive from the community to fix issues with them in the first 10h after a release. So they can also wait 15-20h.

Sounds like you are trying to guess what people who use this CI need it for and when they need it.

I have to point out that this is not a reliable method; and it's also not necessary because you can just ask people and they can tell you.

I wrote this CI because it assists me in watching over the portability of the Sage distribution.
The proposed change is not suitable, that's why we should close this PR to avoid further distractions.

@tobiasdiez
Copy link
Contributor Author

tobiasdiez commented Dec 6, 2023

I wrote this CI because it assists me in watching over the portability of the Sage distribution.

So you looked at a red dots for two months, right? Let me ask you then directly: what did you learned from this about the "portability"?

The proposed change is not suitable, that's why we should close this PR to avoid further distractions.

I've asked you now multiple times (#36610 (comment)), what would be an acceptable execution time for you. I doubt you really need to know the result of the runs exactly 10h after a new release, do you?

Moreover, please don't tag a PR as "invalid" just because you don't agree with the change.

@mkoeppe
Copy link
Contributor

mkoeppe commented Dec 6, 2023

please don't tag a PR as "invalid" just because you don't agree with the change.

Labelling it "invalid" and keeping "needs review" means that I am motioning to close this PR.

@mkoeppe
Copy link
Contributor

mkoeppe commented Dec 6, 2023

So you looked at a red dots for two months, right? Let me ask you then directly: what did you learned from this about the "portability"?

Tobias, that's not a question.
It's an expression of disrespect, and this is not welcome here.

@mkoeppe
Copy link
Contributor

mkoeppe commented Dec 21, 2023

Let's close this.

@tobiasdiez
Copy link
Contributor Author

I've asked you now multiple times (#36610 (comment)), what would be an acceptable execution time for you.

Still requires an answer.

@mkoeppe
Copy link
Contributor

mkoeppe commented Dec 21, 2023

#36726 (comment)

@tobiasdiez tobiasdiez added disputed PR is waiting for community vote, see https://groups.google.com/g/sage-devel/c/IgBYUJl33SQ and removed r: invalid labels Dec 21, 2023
Copy link

Documentation preview for this PR (built with commit 33af375; changes) is ready! 🎉

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 14, 2024

I confirm my vote of -1 on this PR.

It is simply not an improvement.

@kcrisman
Copy link
Member

#36726 (comment)

This does seem reasonably authoritative of an answer (if, as with much right now, going into other issues as well at length). In some sense this isn't even a Sage ticket per se, and even in principle couldn't have been before the Github switch, so it seems that usefulness to those directly using it probably is a compelling argument, unless it's (say) costing the org actual money.


However, can someone (perhaps not the people battling over the invalid tag) explain to me why either option on this particular PR is a hill to die on, even if there is a "battle" regarding this infrastructure elsewhere? I'm not sure it's "invalid" in the sense that sometimes indeed one has to build in series (I have often had to kill a Sage build because I accidentally did make -j4 when I had no business doing so), but then again it also seems like there is no compelling need to make it series either if any slowdown is minimal.

@tobiasdiez
Copy link
Contributor Author

#36726 (comment)

This does seem reasonably authoritative of an answer

Yes, very authoritative.

@kcrisman
Copy link
Member

This does seem reasonably authoritative of an answer

Yes, very authoritative.

Note I didn't say authoritarian. Anyway, obviously I'm not a packaging or CI expert; but the rationale seems compelling to the outsider.


However, more to the point, perhaps it would be helpful to those on the outside of this discussion to have updated reports on whether series is even needed now. Could there be some sort of maximum time condition after which this sort of ticket would be reopened?

As I said above, it's not even really clear whether this is a "Sage" ticket per se. As indicated by the relative lack of interest in it from anyone other than the two main commenters here. Given that, and that it's not clear whether the status quo is better or worse, I'll put my vote at -1 for this, though I hope under different conditions or with other data everyone could come to a different consensus as appropriate.

@tobiasdiez
Copy link
Contributor Author

Which of Matthias arguments in #36726 (comment) do you find the most convincing? Then I will respond to that.

@kcrisman
Copy link
Member

Which of Matthias arguments in #36726 (comment) do you find the most convincing? Then I will respond to that.

Ok:

My solution -- single-sourcing the system packages that are equivalent to our SPKGs, and using them to generate documentation, to generate testing environments (for local testing and CI), and to provide system package advice, ... has been extremely successful. The Sage distribution installs out of the box on a wide range of systems and package configurations. Portability is no longer a weakness of Sage ...

Basically, someone saw a problem in Sage, it was an itch, they scratched that itch, and it seems to work fairly well. Now, as is commonplace in openly developed (not just open source) projects, that person is (de facto, not de jure perhaps) the default person to get to weigh in on what are evidently minor tweaks in how that part runs. (Yes, I'm trusting the assertion that things run well on this very narrow topic now, but on the whole that seems to be the case.) For the record, I don't actually care that much about whether we run the jobs in series or parallel; I'm just so happy there is actually testing on a regular basis that doesn't need manual intervention!

Anyway, that doesn't mean I am suddenly suggesting that there are any BDFLs, even over parts of code, in Sage. It's just a reasonable suggestion that, absent any evidence that a particular issue is truly broken (as opposed to inefficient or not ideally structured), we can all live with a little inefficiency here and there in order to solve the social issue of continuing development motivation. This seems to be one of those cases, maybe only to me, but I suspect to others who (probably wisely) spend their time on fixing bugs and not making comments on tickets on code they don't personally use (i.e. not me).

And I hope @mkoeppe would agree with that sentiment if there are places where, while not ideal, some changes "can be lived with" on his part as well.

@jhpalmieri
Copy link
Member

-1 from me

@tobiasdiez tobiasdiez closed this Oct 30, 2024
@tobiasdiez tobiasdiez deleted the macos-limit-parallel branch October 30, 2024 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: scripts disputed PR is waiting for community vote, see https://groups.google.com/g/sage-devel/c/IgBYUJl33SQ
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants