-
Notifications
You must be signed in to change notification settings - Fork 6
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
Upgrade skill to work with v2 #20
base: master
Are you sure you want to change the base?
Conversation
…imers are now considered active.
…imer rather than int
Hmmm trying to review here but having trouble. I haven't deployed the skill in a while so attempting to start fresh and getting the following on
Not sure how to troubleshoot either of those things. Any thoughts? |
Oh - yes. That happened to me too. You need to move the |
wrt to the first error, I saw that too. I'm not entirely sure the exact steps to get around this issue, but I spent a full day trying to solve it and ultimately I ended up deleting everything and re-deploying from scratch. I'm not convinced that was actually necessary. Some ideas rather than a full rebuild:
|
Yeah initially I had an older, global version of I'm on the same versions 🤔
Will keep poking around... |
Ok got it sorted. My |
One step closer but still not there 😭 Not a terribly useful error message here...
🤷 Also checked in the CloudFormation console and there is no other info there. |
Yeah - I went down this rabbit hole also. I did a lot of looking into the CloudFormation stuff and ultimately I concluded it was a red herring. You might need to redo |
Hm no luck re-configuring. I'll have to hack at this some more later... I initially did this on a different computer and I wonder if that has some magic state I am missing here. |
That's what happened to me :( I was trying to re-create the environment on a new development machine. It might be quicker to go through and delete the skill and lambda functions out of AWS and just try and do a fresh install - at least that's what I did in the end. I'd love to have some steps here that didn't require a full redeploy - but I'm not convinced those changes are tied to the v2 updates. Annoying, but ultimately I don't think the alexa skill has such a high adoption that it matters, and since the skill doesn't really manage any state on its own (it just talks to the django app) it's not ultimately a huge deal to destroy the skill and recreate it until there's a better understanding of what's going on. |
Issue #, if available: #19
Description of changes:
The code was doing an in-line filter for timers with the
active
attribute. With v2, there is no attribute, and all timers returned are considered active. Thus, the loop was filtering all timers and always returning an empty set.Additionally - it looks like nodev12 has been officially removed from support on the Alexa pipeline. I updated the code to use v16 - though the current recommendation is v18. It took me a while to figure out how to even get the codebase off of 12 so I only went to 16, but it's probably a good idea to bump this to 18 sooner than later.
I also took the liberty of fixing 2 minor QOL issues - feedings now parse a float (so you can say 7.5 or whatever). Additionally. one of my last commits created an issue with moment (have I mentioned I hate moment?). This fixes that issue when recording sleep / tummy time.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.