Skip to content

Conversation

@jessfraz
Copy link
Contributor

oxidecomputer/console#716 to prove it works

Signed-off-by: Jess Frazelle <[email protected]>
Signed-off-by: Jess Frazelle <[email protected]>
Signed-off-by: Jess Frazelle <[email protected]>
@jessfraz jessfraz requested review from ahl, davepacheco and smklein March 23, 2022 20:11
jessfraz and others added 2 commits March 23, 2022 13:33
Co-authored-by: David Crespo <[email protected]>
Co-authored-by: David Crespo <[email protected]>
@bnaecker
Copy link
Collaborator

bnaecker commented Mar 23, 2022

Maybe a naive question, but if we're developing the API itself (adding routes, modifying types), how could one test such changes with the CLI?

@jessfraz
Copy link
Contributor Author

jessfraz commented Mar 23, 2022 via email


There's a small demo tool called `./tools/oxapi_demo` that provides a slightly friendlier interface than `curl`, with the same output format. To use the demo, the `node` and `json` programs should be installed and available in the users PATH:
The `oxide` CLI can be installed from the [latest
release](https://github.com/oxidecomputer/cli/releases).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we publishing releases for illumos?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldnt get cargo cross to work for illumos but maybe @jclulow can, its easy to turn on if it compiles

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm reading through https://github.com/cross-rs/cross - I don't think illumos is a supported target by that project.

Manually, I checked out the CLI repo and tried building the x86_64-unknown-illumos target via cross compilation, and am hitting a variety of linker errors (which I think makes sense; it's trying to use my host (Linux) linker).

On the other hand, I can totally check out the CLI repo and build it manually on my Helios machine. If cross-compilation doesn't work, we could add a buildomat job to natively compile + publish the CLI tool?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have opened a PR: oxidecomputer/oxide.rs#117

This should produce an oxide binary for illumos systems. Is there a way, from GitHub Actions, to poll on the completion of a check run from outside GitHub Actions?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dope! hmm not sure, but since I'm uploading things to GCS maybe the release stuff could be changed to wait for everything to be in the bucket before generating things, idk

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@david-crespo
Copy link
Contributor

david-crespo commented Mar 23, 2022

Maybe a naive question, but if we're developing the API itself (adding routes, modifying types), how could one test such changes with the CLI?

You could use oxide api to hit the new route directly, but if you wanted to get fancy you might also want to generate the new OpenAPI spec and plug it into a locally cloned copy of the CLI, re-generate the client, and use that. It doesn't look like there are instructions in the CLI repo for doing that — I'd like to see that too. Do you currently use oxapi_demo for this purpose?

@jessfraz
Copy link
Contributor Author

the cli is also generated from the spec I can add instructions so basically you'd update the spec in the API (oxide.rs) and (cli) repos and go, the cli is a macro from the spec file so its just updating the file.

@bnaecker
Copy link
Collaborator

@david-crespo Yep, I do use oxapi_demo that way, especially for testing user-visible errors, prototyping an integration test I want to build, reminding myself what some of the outputs look like, and probably other things I'm forgetting.

It looks like the oxide api satisfies most or all of those, which is great!

@jessfraz
Copy link
Contributor Author

Added some notes : https://github.com/oxidecomputer/cli/blob/main/README.md#updating-the-spec

@bnaecker
Copy link
Collaborator

Thanks @jessfraz, that's what I was looking for! I'm glad we can still test out the user-facing aspects or just dog-food this repo with a command-line tool, thanks for making sure that's possible!

@jessfraz
Copy link
Contributor Author

to be fair i stole the api command idea from gh who i think stole it from stripe who idk where they got it from..

@bnaecker
Copy link
Collaborator

to be fair i stole the api command idea from gh who i think stole it from stripe who idk where they got it from..

Great artists? Shoulders of giants? Pick your platitude, it's a good idea :)

@jessfraz
Copy link
Contributor Author

if you want to get really fancy too you can do the following, because the alias command allows you to set up aliases:

say you are testing a endpoint like /organizations/{org_name}/projects/{project_name}/snapshots/{snapshot_id}

that's super long right for testing

you can do

oxide alias set get_snapshot 'api /organizations/$1/projects/$2/snapshots/$3'

such that when you run

oxide get_snapshot myorgname myproject my-snapshot-id

it populates the above... anyways if you want to get fancy with it

@jessfraz jessfraz merged commit 9a50adb into main Mar 25, 2022
@jessfraz jessfraz deleted the rm-rf-oxapi_demo branch March 25, 2022 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants