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

Client-Server Split RFC #141

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MagicRB
Copy link

@MagicRB MagicRB commented Dec 16, 2021

Signed-off-by: Magic_RB [email protected]

You can find the text of this RFC here.

@MagicRB MagicRB self-assigned this Dec 16, 2021
@balsoft
Copy link
Member

balsoft commented Dec 16, 2021

At this point might as well rewrite it from scratch. Wouldn't be the first time in this project's life (3 2 1 0)

@MagicRB
Copy link
Author

MagicRB commented Dec 16, 2021

Every time we do so, it improves bit by bit, learning from past mistakes but starting with a clean slate.

@balsoft
Copy link
Member

balsoft commented Dec 16, 2021

As for whether it's actually a good idea? I don't know. When I started writing deploy, the vision I had was of a really simple and dumb script that just does one thing. It turns out even the simplest profile deployment can get complicated once you add all the rollback logic. Maybe rewriting it and splitting it up is the right step. Maybe we should take a step in the other direction and get rid of some extra features (magic-rollback, other fancy activation stuff) in the name of simplicity and reliability of the tool itself, so that there's no need for a separate activate binary, and then implement those features on top using a separate, optional, server. The result should be pretty much the same, but with less rewriting (since we can dumb down the existing codebase quite easily, and then implement the features in a new way on top).

@notgne2
Copy link
Contributor

notgne2 commented Dec 16, 2021

At this point might as well rewrite it from scratch.

in which case having a referenceable specification would be a good thing!

I also kindof wanted to do a lot of rewriting already, I've been pondering some issues and limitations of magic-rollback and came to the conclusion that most of that logic has to be redesigned from scratch, and could probably convert a lot of my thoughts to a similar RFC too


I propose to split `deploy-rs` into a server and a client, `deploy` and `activate` respectively. Basically, while now
`activate` is just a helper, this RFC proposes to make it a self standing program. The basic jist is that the two
components would communicate over a open ssh connection, using plain REST (this is important later). That would allow us
Copy link
Member

Choose a reason for hiding this comment

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

We'd have to make a new ssh connection to confirm activation anyways, since that's precisely what we're confirming (the ability to ssh into the machine).

Also, we'd need to reimplement nix copy for this. Should not be too difficult, but something to keep in mind.

Copy link
Author

Choose a reason for hiding this comment

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

Well we'd still be down to 2 ssh connections from 3 or 4 or however many we have now

@notgne2
Copy link
Contributor

notgne2 commented Dec 16, 2021

Rollback stuff makes it complicates yes, but at least after our current iteration of the codebase we know how it makes it complicated, so if we include it again we are aware how to implement it more cleanly without creating annoying logic problems and disrupting the readability of the rest of the code

@MagicRB
Copy link
Author

MagicRB commented Dec 16, 2021

in which case having a referenceable specification would be a good thing!

If I had to describe how I'd imagine a perfect future for this RFC and deploy-rs/yeet generally is that before writing a single line of code, we go though all the code we have now, write down all the features we have and then analyze what each feature encompasses, the requirements and such. Create api specifications. Then as the last step we just write code which compiles and works on the first attempt :D.

@MagicRB
Copy link
Author

MagicRB commented Dec 16, 2021

Maybe rewriting it and splitting it up is the right step. Maybe we should take a step in the other direction and get rid of some extra features

How would that work? Because we need a smarter program on the remote machine to do cooler things, so I can't really imagine a way where we build something complex on a simpler base than we have now. Maybe plugins on both ends?

The way I imagined it is that when you run yeet in single-client-mode, it installs the server component and then after its done removes itself from the target automatically, the complex nature would be completely hidden from the user in that case.

@MagicRB
Copy link
Author

MagicRB commented Dec 17, 2021

should I expand on the proposal? I'd start with a detailed functionality description and feature set.

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.

3 participants