Fixes for issues #1000 and #1002 #1005
Conversation
…heduler" arg, required to spawn a new process
…ted Module-Format to wasm64
|
Thanks @av1ctor I addressed some of these issues in #1004. You may want to rebase your branch onto main so that you can get those changes. Your conflicts should be resolved and your PR should end to be much smaller as a result. Also, there's no need to manually bump version numbers, as that is done using CI. |
|
@TillaTheHun0 Ok, I merged the PR with your changes. Thanks! |
There was a problem hiding this comment.
Thank you!
There a couple of things that don't belong in this PR that I recommend removing entirely, or moving to a separate PR.
There's also a couple things missing from adding scheduler as an option on the CLI (thank for this btw, it seems to be almost there)
| @@ -12,5 +14,7 @@ export const AoModuleTags = [ | |||
| DEFAULT_INPUT_ENCODING_TAG, | |||
| DEFAULT_OUTPUT_ENCODING_TAG, | |||
| DEFAULT_VARIANT_TAG, | |||
| DEFAULT_MEMORY_LIMIT_TAG, | |||
| DEFAULT_COMPUTE_LIMIT_TAG, | |||
There was a problem hiding this comment.
This shouldn't be in this PR. There needs to be internal discussion on if there should even be defaults for this or if it should be derived from the build flags used to produce the wasm binary (see #983)
Let's keep this PR small and focused on only the issues highlighted in the issues it claims to address.
| .usage('-l c <my-project-name>') | ||
| .option( | ||
| '-l, --lang <language:string>', | ||
| 'The starter to use. Defaults to Lua. Options are "lua" and "cpp"' | ||
| 'The starter to use. Defaults to Lua. Options are "lua" and "c"' |
There was a problem hiding this comment.
This should not be in this PR. If anything, it should be a separate issue and PR, so that our team can look at it, by itself.
| @@ -165,12 +165,13 @@ export const uploadModuleWith = | |||
| */ | |||
| export const spawnProcessWith = | |||
| ({ walletExists, readWallet, create }) => | |||
| async ({ walletPath, module, tags }) => { | |||
| async ({ walletPath, module, scheduler, tags }) => { | |||
| if (!(await walletExists(walletPath))) throw new WalletNotFoundError() | |||
|
|
|||
| const wallet = await readWallet(walletPath) | |||
| const res = await create({ | |||
There was a problem hiding this comment.
create in ao-spawn.js needs to be updated to accept scheduler field on the argument.
| function schedulerArgs (src) { | ||
| return [ | ||
| '-e', | ||
| `SCHEDULER_ADDRESS=${src}` |
There was a problem hiding this comment.
I don't see where this is used in ao-spawn.js. I would suspect that it would be mapped to scheduler when calling uploadAoProcess, similar to WALLET_PATH and MODULE_TX
| .option( | ||
| '-s, --scheduler <address:string>', | ||
| 'the ao scheduler address', | ||
| { required: true } | ||
| ) |
| "warp-arbundles": "^1.0.4", | ||
| "wasm-metering": "permaweb/wasm-metering#v0.2.2" |
There was a problem hiding this comment.
Not sure what this is about. Can it be removed?
Fixes for issues #1000 and #1002