Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

add macOS 10.14 Mojave to Azure Pipelines CI #804

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

Merged
merged 1 commit into from
Nov 4, 2019
Merged

add macOS 10.14 Mojave to Azure Pipelines CI #804

merged 1 commit into from
Nov 4, 2019

Conversation

CameronTStark
Copy link
Contributor

@CameronTStark CameronTStark commented Oct 29, 2019

  • Closes Add os x tests to azure pipelines config #719
  • I am familiar with the contributing guidelines.
  • Fully tested. Added and/or modified tests to ensure correct behavior for all reasonable inputs. Tests (usually) must pass on the TravisCI and Appveyor testing services.
  • Updates entries to docs/sphinx/source/api.rst for API changes.
  • Adds description and name entries in the appropriate docs/sphinx/source/whatsnew file for all changes.
  • Code quality and style is sufficient. Passes LGTM and SticklerCI checks.
  • New code is fully documented. Includes sphinx/numpydoc compliant docstrings and comments in the code where necessary.
  • Pull request is nearly complete and ready for detailed review.

Adds Azure test functionality for the macOS 10.14 Mojave system. This implementation was simple (for now) and replicates the standard Linux job.

I wanted to ask you @wholmgren if we should be mindful of increasing the time duration of Azure Pipelines with another three test cycles. This may lengthen each iteration a contributor must wait after a change so I think it's worth considering.

If it is a concern, would it a better use of time be to only test the "bare_linux" version and not the 'Test_conda_linux'? I would argue that the "bare_linux" is the least stable from an infrastructure POV. I'd also guess, and I could be completely wrong, that most people that choose to run on Linux aren't going the Anaconda route.

If that's not the case and we want to be sure about Anaconda installs would we then add a 'Test_conda_macOS' job? If so, we'd be increasing the CI time by 66%. =S

@wholmgren
Copy link
Member

I'm a little reluctant to drop the conda_linux configuration because we previously ran into an issue that was specific to how continuum compiled scipy for linux. I do use conda environments on linux because it's an easy way for me to manage custom python environments without root access.

Is a conda_mac configuration more representative of end user environments than a bare_mac configuration? I don't think we need both.

@mikofski
Copy link
Member

can we spread the load over several CI's from different providers, some travis, some azure, and some appveyor or circle? what are their free offerings? Does azure offer free concurrency for oss? I'll try to answer these if useful

@wholmgren
Copy link
Member

I've thought it would be easier to maintain the CI if we reduce to just 1 provider. They all have their quirks, but lately Azure has been my favorite. In this PR, Azure completes 12 configurations across 3 platforms in roughly the time it takes Travis to complete 4 configurations on 1 platform.

@CameronTStark
Copy link
Contributor Author

I'm a little reluctant to drop the conda_linux configuration because we previously ran into an issue that was specific to how continuum compiled scipy for linux. I do use conda environments on linux because it's an easy way for me to manage custom python environments without root access.

I'm not familiar with the severity of that issue you had before but if it was a "one-off" occurance I would advocate for a quicker CI experience than covering than for trying to cover all avenues of usability all the time. Having to wait an extended amount of time for the CI through every iteration of a PR could scare off new contributors and a drag on the frequent contributors. We can always spin up a wider scoped CI on our own branch if an issue comes up with another build that's in heavy use.

You've earned plenty of wisdom in this area so I'd defer to you from a maintainers perspective. The rest of the team may have a strong opinion one way or another as well.

Is a conda_mac configuration more representative of end user environments than a bare_mac configuration? I don't think we need both.

This may be a good question for a survey. I personally use Homebrew on Mac which shields the user from using the system Python and doesn't require sudo. I feel like conda is another layer of abstraction that's not necessary for *nix systems but worthwhile on Windows.

@wholmgren
Copy link
Member

I don't have a strong preference between bare_mac, conda_mac, or both.

I'm not that concerned about total CI speed so long as results begin to be returned quickly. The vast majority of errors are caught within a single platform's bare (~1 min) or a single platform's conda (~4 min) configurations, and then the total build is immediately marked failed. That's faster than any of the Travis results (~6 minutes). LGTM is always the slowest (typically 30-60 minutes).

@CameronTStark
Copy link
Contributor Author

I don't have a strong preference between bare_mac, conda_mac, or both.

Let's just keep it bare_mac for now. If we hear that everyone is using conda on mac and/or there's a significant difference in bug occurrence then we can add it in.

I'm not that concerned about total CI speed so long as results begin to be returned quickly.
I wasn't aware of the "fail fast" mode. From that perspective, adding tests shouldn't be as big of a deal especially with LGTM making us wait significantly longer as well.

Thanks for the feedback!

@wholmgren wholmgren added this to the 0.7.0 milestone Nov 4, 2019
Copy link
Member

@wholmgren wholmgren left a comment

Choose a reason for hiding this comment

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

Sounds good. Thanks!

@wholmgren wholmgren merged commit 209e8d8 into pvlib:master Nov 4, 2019
wholmgren added a commit that referenced this pull request Nov 5, 2019
@CameronTStark CameronTStark deleted the azure_mac branch November 7, 2019 03:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add os x tests to azure pipelines config
3 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.