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

Jump to bottom

nixos/buildbot-master: support reporters, migrate away from status #91043

Merged
merged 1 commit into from Jun 18, 2020
Merged

nixos/buildbot-master: support reporters, migrate away from status #91043

merged 1 commit into from Jun 18, 2020

Conversation

flokli
Copy link
Contributor

@flokli flokli commented Jun 18, 2020

Since Buildbot 0.9.0, status targets were deprecated and ignored.
There's a very small line on startup explaining that, and status simply
isn't reported. Avoid others the same headaches, and do it right in the
NixOS module.

As there might have been changes in the way reporters are organized, and
configuration might need to be migrated remove the old option, and not
just provide an alias.

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@flokli flokli requested review from lopsided98 and makefu June 18, 2020 14:42
makefu
makefu approved these changes Jun 18, 2020
View reviewed changes
Copy link
Contributor

@makefu makefu left a comment •
edited

Choose a reason for hiding this comment

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

LGTM, have not tested the module itself though.

i will try the buildbot test.

@flokli
Copy link
Contributor Author

flokli commented Jun 18, 2020 •
edited

Umm, the key is actually called services. I'll test another configuration here in will get back to it shortly.

@flokli
nixos/buildbot-master: support reporters, migrate away from status
9538bf5 
Since Buildbot 0.9.0, status targets were deprecated and ignored.
There's a very small line on startup explaining that, and status simply
isn't reported. Avoid others the same headaches, and do it right in the
NixOS module.

As there might have been changes in the way reporters are organized, and
configuration might need to be migrated remove the old option, and not
just provide an alias.
@flokli
Copy link
Contributor Author

flokli commented Jun 18, 2020 •
edited

So, while this should go into the services key, it's labelled in the docs as "Reporters", so IMHO we should keep calling the NixOS attribute reporters.

If upstream some day decides to put more than Reporters in services, we can still do this transparently, but if we call this services now I'd assume it'd become a mess.

I gave this a spin, and Buildbot successfully picked up the reporter I defined in services, so LGTM.

@flokli
Copy link
Contributor Author

flokli commented Jun 18, 2020

@GrahamcOfBorg test buildbot

@flokli flokli requested review from jonringer and FRidh June 18, 2020 16:41
@makefu
Copy link
Contributor

makefu commented Jun 18, 2020

It seems that buildbot does not even build on master:
ERROR: No matching distribution found for botocore<1.17.0,>=1.16.23 (from boto3==1.13.23)
At least this is the error i get when running nix-build nixos/tests/buildbot.nix

@flokli
Copy link
Contributor Author

flokli commented Jun 18, 2020

That's another failure, which should be covered by the boto* bumps in staging-next.

This PR was based on a previous version of master, and running the tests by checking out this specific commit does work.

@flokli flokli mentioned this pull request Jun 18, 2020
Merged
10 tasks
@makefu
Copy link
Contributor

makefu commented Jun 18, 2020

That is good enough for me, i trust you :)

@flokli flokli merged commit e051dab into NixOS:master Jun 18, 2020
@flokli flokli deleted the buildbot-reporters branch June 18, 2020 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@makefu makefu makefu approved these changes

@lopsided98 lopsided98 Awaiting requested review from lopsided98

@jonringer jonringer Awaiting requested review from jonringer

@FRidh FRidh Awaiting requested review from FRidh

Assignees
No one assigned
Labels
6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 10.rebuild-linux: 1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@flokli @makefu