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

Release Resin-SDK v7.0.0 #420

Merged
merged 23 commits into from Oct 12, 2017

Conversation

Projects
None yet
2 participants
@pimterry
Member

pimterry commented Oct 12, 2017

This includes lots of breaking changes, notably including the upgrade to the new API v3.

All these changes have previously been reviewed, this PR is just merging them all together and using versionbot to actually ship them to npm.

For posterity, this PR includes:

  • #398
  • #407
  • #410 (plus some changes after rebasing over #419)
  • #417
  • #418
  • a couple of last extra changes and bugfixes on the end
    ---- Autogenerated Waffleboard Connection: Connects to #358

pimterry added some commits Sep 21, 2017

Merge pull request #398 from resin-io/358-discontinued-device-types
Breaking: Don't allow creating devices with discontinued device types
*BREAKING*: Remove application.online_devices and application.device_…
…length

These were slightly convenient, but hacky and inconsistent with any data
retrieved manually with $expands. One day we might bring these back, but
for now it's much cleaner for everybody if we keep this simple.

Change-Type: major
*BREAKING*: Remove device.application_name and device.dashboard_url
Both of these were generated from expanded data we'd like to remove, and
somewhat messily at that. You can easily generate them by hand by
expanding application yourself and looking at the name and
id there by hand.

Change-Type: major
*BREAKING*: Don't expand relationships by default. Pass { expand: '..…
….' } options to opt in instead.

Change-Type: major
Merge pull request #410 from resin-io/remove-generate-api-key
Replace generateApiKey with generateProvisioningKey
*BREAKING*: Stop actively supporting node 4.
Node 4 may well still work with the SDK for quite a while, but we'll no
longer actively test against it, and it's quite possible that it may
stop working entirely in any future release.

Change-Type: major
Merge pull request #417 from resin-io/deprecate-node-4
*BREAKING*: Stop actively supporting node 4.
*BREAKING*: Upgrade to API v3. Main change is that all relationships …
…& result properties now include verbs (e.g. device.application is now device.belongs_to__application).

Change-Type: major

@pimterry pimterry requested a review from thgreasi Oct 12, 2017

@thgreasi

This comment has been minimized.

Show comment
Hide comment
@thgreasi

thgreasi Oct 12, 2017

Member

Woohoo! I'm on it!

Member

thgreasi commented Oct 12, 2017

Woohoo! I'm on it!

@pimterry

This comment has been minimized.

Show comment
Hide comment
@pimterry

pimterry Oct 12, 2017

Member

@thgreasi I've been doing my own review of the codebase - I think I've actually found a couple of bugs in here in the untested device methods (things that do stuff with real devices, like shutdown/reboot) from the combination of changing the relationships and removing the expands. I'm going to do some quick testing to confirm, fix them, and then update this PR with the results soon.

Member

pimterry commented Oct 12, 2017

@thgreasi I've been doing my own review of the codebase - I think I've actually found a couple of bugs in here in the untested device methods (things that do stuff with real devices, like shutdown/reboot) from the combination of changing the relationships and removing the expands. I'm going to do some quick testing to confirm, fix them, and then update this PR with the results soon.

@resin-io-versionbot

This comment has been minimized.

Show comment
Hide comment
@resin-io-versionbot

resin-io-versionbot bot Oct 12, 2017

Contributor

@pimterry, status checks have failed for this PR. Please make appropriate changes and recommit.

Contributor

resin-io-versionbot bot commented Oct 12, 2017

@pimterry, status checks have failed for this PR. Please make appropriate changes and recommit.

@@ -982,7 +989,7 @@ getDeviceModel = (deps, opts) ->
baseUrl: apiUrl
body:
deviceId: device.id
appId: device.application[0].id
appId: device.belongs_to__application[0].id

This comment has been minimized.

@thgreasi

thgreasi Oct 12, 2017

Member

Like above

@thgreasi

thgreasi Oct 12, 2017

Member

Like above

data:
appId: device.application[0].id
appId: device.belongs_to__application[0].id

This comment has been minimized.

@thgreasi

thgreasi Oct 12, 2017

Member

Like above twice

@thgreasi

thgreasi Oct 12, 2017

Member

Like above twice

@@ -1071,7 +1078,7 @@ getDeviceModel = (deps, opts) ->
baseUrl: apiUrl
body:
deviceId: device.id
appId: device.application[0].id
appId: device.belongs_to__application[0].id

This comment has been minimized.

@thgreasi

thgreasi Oct 12, 2017

Member

Like above and actually I think that's the same for the rest of the occurrences in this file.
Sorry for the noise.

@thgreasi

thgreasi Oct 12, 2017

Member

Like above and actually I think that's the same for the rest of the occurrences in this file.
Sorry for the noise.

@pimterry pimterry requested a review from thgreasi Oct 12, 2017

@pimterry

This comment has been minimized.

Show comment
Hide comment
@pimterry

pimterry Oct 12, 2017

Member

@thgreasi See the last 4 commits - I've been through and manually tested every untested method in the devices file with a real pi. I've fixed the few that were broken by missing relationships, plus fixing a couple of preexisting bugs and mess in some of the others.

I've also stopped exposing the weird ensureSupervisorCompatibility method (though we still use it internally), which just did a semver comparison and and then threw an exception, and didn't really know anything about supervisor versions at all.

Rereview please!

Member

pimterry commented Oct 12, 2017

@thgreasi See the last 4 commits - I've been through and manually tested every untested method in the devices file with a real pi. I've fixed the few that were broken by missing relationships, plus fixing a couple of preexisting bugs and mess in some of the others.

I've also stopped exposing the weird ensureSupervisorCompatibility method (though we still use it internally), which just did a semver comparison and and then threw an exception, and didn't really know anything about supervisor versions at all.

Rereview please!

@thgreasi

Have some questions but already LGTM 👍 🎊

return request.send
method: 'DELETE'
method: 'POST'

This comment has been minimized.

@thgreasi

thgreasi Oct 12, 2017

Member

I guess that this changed at some point

@thgreasi

thgreasi Oct 12, 2017

Member

I guess that this changed at some point

This comment has been minimized.

@pimterry

pimterry Oct 12, 2017

Member

As far as I can tell, it's needed this since basically forever, so it's never worked... The API supervisor proxy requires a POST, plus the real method to use in the body.

Horia fixed the same issue in ping recently. The problem is we just don't have automated tests for this stuff right now, so any useful but rarely used methods like this that require real devices can be silently broken 😢

@pimterry

pimterry Oct 12, 2017

Member

As far as I can tell, it's needed this since basically forever, so it's never worked... The API supervisor proxy requires a POST, plus the real method to use in the body.

Horia fixed the same issue in ping recently. The problem is we just don't have automated tests for this stuff right now, so any useful but rarely used methods like this that require real devices can be silently broken 😢

@@ -1070,8 +1085,13 @@ getDeviceModel = (deps, opts) ->
# if (error) throw error;
# });
###
exports.update = (uuidOrId, options, callback) ->
exports.get(uuidOrId).then (device) ->
exports.update = (uuidOrId, options = {}, callback) ->

This comment has been minimized.

@thgreasi

thgreasi Oct 12, 2017

Member

What happens when updating with an empty object? Does it serve like a ping?

@thgreasi

thgreasi Oct 12, 2017

Member

What happens when updating with an empty object? Does it serve like a ping?

This comment has been minimized.

@pimterry

pimterry Oct 12, 2017

Member

Update options are actually only allowed to be force: true/false (or nothing). All this method does is ping a device to get it to check for an update. I'm not 100% sure why that's useful, but it's a feature the supervisor supports, so I'm happy to leave it here for now.

@pimterry

pimterry Oct 12, 2017

Member

Update options are actually only allowed to be force: true/false (or nothing). All this method does is ping a device to get it to check for an update. I'm not 100% sure why that's useful, but it's a feature the supervisor supports, so I'm happy to leave it here for now.

@@ -101,7 +101,7 @@ getDeviceModel = (deps, opts) ->
# console.log('Is compatible');
# });
###
exports.ensureSupervisorCompatibility = ensureSupervisorCompatibility = Promise.method (version, minVersion) ->
ensureSupervisorCompatibility = Promise.method (version, minVersion) ->

This comment has been minimized.

@thgreasi

thgreasi Oct 12, 2017

Member

It seems that we don't use it in the dashboard, so 👍

@thgreasi

thgreasi Oct 12, 2017

Member

It seems that we don't use it in the dashboard, so 👍

@resin-io-versionbot

This comment has been minimized.

Show comment
Hide comment
@resin-io-versionbot

resin-io-versionbot bot Oct 12, 2017

Contributor

VersionBot failed to carry out a status check for the above pull request here: #420. The reason for this is:
2 of 5 required status checks are pending.
Please carry out relevant changes or alert an appropriate admin.

Contributor

resin-io-versionbot bot commented Oct 12, 2017

VersionBot failed to carry out a status check for the above pull request here: #420. The reason for this is:
2 of 5 required status checks are pending.
Please carry out relevant changes or alert an appropriate admin.

@pimterry

This comment has been minimized.

Show comment
Hide comment
@pimterry
Member

pimterry commented Oct 12, 2017

Here we go!

@resin-io-versionbot resin-io-versionbot bot merged commit 25db78a into master Oct 12, 2017

6 checks passed

AutoMerges PR merging is in progress
Reviewers 1/1 review approvals met
Versionist Found all required commit footer tags
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@resin-io-versionbot resin-io-versionbot bot deleted the v7 branch Oct 12, 2017

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