-
Notifications
You must be signed in to change notification settings - Fork 47
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
Make Resin-SDK builds gentler on the API #241
Comments
@emirotin @jviotti @Page- I think we should:
Thoughts? |
Agree to both things (especially if we can solve the issue with npm@4)
…On Thu, Dec 15, 2016 at 1:23 PM, Tim Perry ***@***.***> wrote:
@emirotin <https://github.com/emirotin> @jviotti
<https://github.com/jviotti> @Page- <https://github.com/Page-> I think we
should:
-
Stop building on pushes and instead just build on PRs
-
Find a way to change the NPM config + build config so we only run the
test suite once per job. I've got a plan for a reasonably nice solution to
this that doesn't cause problems for pre-NPM4 users - I'll push a strawman
in a minute.
Thoughts?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#241 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAgGCC4LEaCdAhUwVlBRklBLgBD44pM7ks5rIRU0gaJpZM4LN7LS>
.
--
Eugene Mirotin
Senior Frontend Engineer
site: Resin.io <https://resin.io/>, twitter: @resin_io
<https://twitter.com/resin_io>
|
Hi @pimterry , I made sure you have access to Appveyor by including it into the CLI/SDK GH team (which is explicitly added to Appveyor). After looking a bit around, I think the only way to accomplish what you're trying to do is by adding the following to branches:
only:
- master This will prevent builds for branches, but it looks that it will still build PRs (according to the docs). Give that a go and let me know if it still doesn't work, and we can get in touch with Appveyor's priority support. |
@jviotti Thanks! That pointer was enough to get me to the config option I was looking for: appveyor/ci#882. #244 turns this on. Sorry for the PR flood! With that one though we should hopefully have most of these build issues in the SDK solved, for now 😄 |
All done - we now run 25% of the tests we were running before, so I'm going to close this. Should drastically improve things. If we need to go further in future, we'll need to substantially refactor the actual flow of the tests, but I suggest we leave that unless we see more problems that'd make it necessary. |
Currently if we're making lots of changes to the Resin-SDK, we end up flooding our API with traffic from ongoing builds: https://www.flowdock.com/app/rulemotion/resin-devops/threads/kyclI6am1xCvhpSjCOSv4HntTkQ.
It would be good to avoid this. As discussed elsewhere we currently duplicate a lot of testing work, which makes this far worse:
Some of this is useful, but 1 and 4 at least are not, and stopping those would reduce API load (and also build waiting times) by 75%.
In addition, the tests do many costly operations far more often than they appear in real world usage: creating new users, creating new applications. We could refactor the tests to do this less.
The text was updated successfully, but these errors were encountered: