Skip to content
This repository has been archived by the owner on Oct 31, 2023. It is now read-only.

No lattice repl #107

Merged
merged 11 commits into from Apr 16, 2021
Merged

No lattice repl #107

merged 11 commits into from Apr 16, 2021

Conversation

brooksmtownsend
Copy link
Member

@brooksmtownsend brooksmtownsend commented Mar 5, 2021

Addresses #92

No-nats mode now supports all but the following commands from the standard lattice connected mode:

  1. ctl get claims
  2. ctl update actor
  3. ctl get inventory (partial)
    These are all unsupported at the moment because there are not equivalent commands to invoke on the host directly, and some make use of the host controller on the control interface side in order to return the correct information. This will need to be addressed. I'm primarily concerned about implementing get inventory for the host inventory, as that contains much needed information, specifically link names and OCI references. @autodidaddict I've added what I can of support for now (see the host inventory implementation for standalone) however we might be better off implementing some of this functionality on the host side.

For this PR, I've added a few additional threads and channels to allow wash up to run a REPL without connecting to NATS, as indicated by the Standalone title by the REPL. This launches an embedded host, as usual, however, commands will be invoked directly on that host instead of proxying through the control interface.

A few modifications for reviewers:

  • ctl and up are getting to contain large amounts of code, so I've split some large pieces into separate files under a folder of the same name. This is a pretty standard Rust pattern. I didn't split larger pieces of the REPL out into separate files because I made many assumptions about accessing fields, and I'd have to write just as much code as I'd save in getters, setters, and modifications.
  • the output module under ctl/output.rs is used to standardize the output format for control interface operations and embedded host operations, that way what the user sees should be identical for standalone mode and lattice connected mode.
  • I edited some integration tests to better check for failures while I was refactoring the output modules.

@autodidaddict
Copy link
Member

Safe? You think the weekend makes you safe?
image

@autodidaddict
Copy link
Member

Maybe the diff isn't showing it, or I'm confused. I was looking for the code that uses channels to send commands either to the control interface or to the managed host.. is that somewhere else?

@brooksmtownsend
Copy link
Member Author

@autodidaddict Yeah it's just moved around because I restructured the project a bit to clean up the code, check src/up/mod.rs https://github.com/wasmCloud/wash/pull/107/files#diff-dcf780801376c219d3b5d7f2f9a5a2745c5ccb124b8c621281e13d3c775ed9f3

@autodidaddict
Copy link
Member

Thanks for the link.. though it's still not in the diff, this is what I was looking for:

if let Ok(ctlcmd) = host_op_receiver.try_recv() {

autodidaddict
autodidaddict previously approved these changes Mar 10, 2021
implemented ctl get hosts for standalone host, channels abound

comments and host command scaffold

modified all the things. can start and call actor

implemented start, stop and link commands

moved output helpers to its own file

addressed clippy warnings, added warnings for partial/no support

remove todo that shouldnt be done

capitalizing host id
@brooksmtownsend
Copy link
Member Author

brooksmtownsend commented Apr 13, 2021

As of my latest commit:

  • Fixed bug where characters would show vertically across output window
  • Implemented the above three functions, which now brings standalone mode to full parity with lattice connected mode

There are only a few more areas where I need to implement functionality wasmcloud-host:

updated to display single image_ref
@brooksmtownsend
Copy link
Member Author

All functionality has been implemented and wash no-nats (standalone) mode has exact parity with nats (lattice) mode. The build is going to fail because of relative path dependencies and that can be remedied once wasmCloud/wasmCloud#163 is ready

@brooksmtownsend brooksmtownsend marked this pull request as ready for review April 16, 2021 13:41
@brooksmtownsend
Copy link
Member Author

REVIEWER'S GUIDE TO SOME OF THE BIG CHANGES (@autodidaddict )

I've refactored the ctl and up modules so they appear to have been deleted and written. The ctl module has been refactored into mod and output, where mod is the functionality of the ctl module and output is where all of the helper functions exist to translate the CTL resources into a String for output in the terminal. The neat thing about this is that I'm using these helper functions with the standalone mode as well, that way there will be no difference between what the user sees in either mode.

up was refactored as well, mostly to pull some of the large parts of standalone mode types into another file. I'd like to continue to refactor this a bit as up and ctl are approaching 1000 lines but for now this is enough change.

Copy link
Member

@autodidaddict autodidaddict left a comment

Choose a reason for hiding this comment

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

LGTM once clippy has been placated.

CLIPPY! (tests)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants