Skip to content
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

Adding the export functionality #13

Open
wants to merge 29 commits into
base: main
Choose a base branch
from
Open

Conversation

rogeruiz
Copy link
Owner

@rogeruiz rogeruiz commented Oct 30, 2022

Adding the basic functionality for the export method. This addresses the issues
brought up in #11.

Acceptance criteria

  • Adding in the export library
  • Figure out how to launch Structurizr-CLI
    • Have Export leverage the Structurizr-CLI Docker image
    • Have Export leverage calling the Java code in Structurizr-CLI
  • Export validates the Structurizr-DSL file that gets fed to the export via
    --workspace.
  • Export validates the PlantUML that gets generated

This patch adds the ability to call the Structurizr-CLI from within the
Rust code. It's not working as expected. It seems like the response from
the Docker Exec is getting cached? This might not be an issue if I
implement the ability for the Docker call to read dynamic input and if
the output is updated every time I run it. So we'll see. But for now, I
think I need to understand more what Bollard is doing or use some other
Crate for the Docker API.
After reading through the helpful Cargo Book, I'm realizing that I
didn't fully understand the benefits or how to properly modularize my
Rust code. This is the initial work to bring everything under a single
binary release. I started down this path because I want to remove the
caching I think I'm experiencing on subsequent runs of the Docker Exec
commands for Structurizr-CLI. So far, I'm still working through this a
bit.

[=> Link to the Cargo Book that explains some of this stuff.](https://doc.rust-lang.org/cargo/guide/project-layout.html)
So I think what's happening here is that Clap's subcommand function is
finishing before the async code that Bollard is running is able to
complete. This closes the STDOUT and prevents me from seeing the
message. I'll figure it out just not tonight.
@rogeruiz
Copy link
Owner Author

rogeruiz commented Nov 2, 2022

Got stuck tonight on the Async and Sync nature of things. We'll see how it pans out. I'm going to read this book for now.

https://rust-lang.github.io/async-book/01_getting_started/03_state_of_async_rust.html#compatibility-considerations

This ends up ignoring errors which is okay because I don't return any
yet from the `run_export` function.
Wow! So this really took some effort and frustration. I finally have a
working example. I'll be reviewing this patch and making line comments
around what these changes are and how to improve the output. But I got
it working thankfully.
Sometimes this is all the time you get ⏲️
The initial MVP is about supporting PlantUML renders only. I am
including Mermaid, as it's something that won't use the `render`
subcommand.
The `zzz` crate wasn't helping me here. I couldn't really figure out how
to configure. But `terminal-spinners` and `termcolor` I can use to set
colors and progress indicators in an much easier way.
I'm hoping by making this constraint, I get errors in my CLI handled if
the string that gets passed in isn't an actual path.
This will need to be documented better. I want to add some tests. But I
was able to use this export command to build a bunch of PlantUML files
locally. Very excited!
This is some basic stuff in Rust, I'm learning, but the compiler can be
so helpful that it's actually a little noisy.
This error handling is checking that the file that gets passed in as
workspace is a `.dsl` file and that it is an actual file that actually
exists. The error messaging needs to be improved for sure. I'm going to
be try to leverage the `anyhow` crate for this in the future.
Previously, this was a `f` variable which wasn't very legible. I'm
updating this so that it follows a convention of `valid_*` before
passing these `&str` to the `run_export()` function. I like this
approach better as it allows me to allow for some testing in the main.rs
file to verify my own logic around this stuff. But that's a ways off at
this point. For now the work is to get this working and have variables
be legible to me later.
I'm not sure how to use it right now, but I'll get there. This is just
to make sure I remember to use it.
Since `.dsl` is the only supported extension here so errors should show
that.
@rogeruiz
Copy link
Owner Author

rogeruiz commented Feb 5, 2023

More thoughts on error handling here from the Rust Discord.

make your own struct around a pathbuf and use the try_from thing

@rogeruiz
Copy link
Owner Author

rogeruiz commented Feb 5, 2023

From reading more on Structs and TryFrom, I don't think that's going to be the best thing to do here.

https://doc.rust-lang.org/stable/rust-by-example/conversion/try_from_try_into.html

https://doc.rust-lang.org/std/keyword.impl.html

I do think a custom Structs and some implementation functions make sense here for ergonomics. Especially since I'll be using this logic in two command to start.

I think I'll need to figure out how to do this for Clap errors, but
maybe not. For now I just want some consistency.
This is some simple checking for an output directory. The logic here is
simple and doesn't care to see if a file path was passed in rather than
a directory path. The error handling is generic enough that I hope folks
using the Scuttle-CLI can figure it out.
Match statements can be simplified to return rather than create an
assignment here since the arms for the match will return what I feed
into them.
@rogeruiz
Copy link
Owner Author

rogeruiz commented Mar 2, 2023

While testing this with @felipe-lee, I found that I don't have good error handling for when the Docker socket is not actively running on a users machine. Thanks, Felipe!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

1 participant