WIP: Assets plugin #230
WIP: Assets plugin #230
Conversation
The commands assets:{init, upload, list} work in basic ways right now. Needs documenting, refactoring, testing, etc.
Prints nice ending output. Puts most logging into debug and is more vague with the spinner logging.
not just the last build
|
I think this looks great overall and I'm happy to release it alongside the rest. The only thing I'd say is that because it's not a standalone like |
|
Ah yes, I did mean to include those bits. I'm actually holding off including this until twilio-cli-core#118 is deployed with the CLI this week. That way I can dump the I meant to add tests too 😄 I can get to them after v1 though. |
|
I think this is ready for a real review now. It works with all the basic Twilio CLI flags and while it does still lack tests, it works! |
packages/plugin-assets/src/init.js
Outdated
| } catch (error) { | ||
| logger.debug(error.toString()); | ||
| throw new TwilioCliError( | ||
| `Could not fetch asset service environment with config: |
There was a problem hiding this comment.
It would be useful to provide some steps here as to why / how to fix it
There was a problem hiding this comment.
I've added a message to run in debug mode for more detail. Many of the errors throughout these scripts should not occur unless there is a network error or the user has edited the config file by hand and changed the SIDs.
packages/plugin-assets/src/upload.js
Outdated
| newAsset.sid = existingAsset.sid; | ||
| spinner.start('Overwriting existing asset'); | ||
| } else if (answers.conflict === 'Rename') { | ||
| let newNameAnswers = await inquirer.prompt([ |
There was a problem hiding this comment.
We should do verification here that it doesn't contain invalid characters
There was a problem hiding this comment.
Invalid characters are ;,?:@+&$()' " and fragments, so #.
Most of these can actually be uri encoded and still work fine. Rather than confuse the code (we should also check filenames which are passed in as the original file, for example) for an edge case I would prefer for now to fail the asset version upload with a better error message (I've added the asset path to the failure message).
We can add this sort of checking in later, if required.
packages/plugin-assets/src/upload.js
Outdated
|
|
||
| const spinner = ora(); | ||
|
|
||
| const upload = async ({ |
There was a problem hiding this comment.
Can you break the different steps in this function into individual functions? Just makes reading this and debugging it easier.
This is a new Twilio CLI plugin that can create a new Twilio Runtime Service to be used as a bucket to store assets. You can then upload assets to the bucket and list the currently available assets.
I am starting a draft PR to get feedback on the code and on how is best to release this. Should we get it out as a version 1 alongside the major release of all other serverless toolkit packages?
It includes some changes to serverless-api, though these are really just exporting a couple of functions that weren't previously exported.
Contributing to Twilio