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

Improve docker-compose example to bypass setup command #33

Open
oguzhanunlu opened this issue Nov 1, 2019 · 6 comments
Open

Improve docker-compose example to bypass setup command #33

oguzhanunlu opened this issue Nov 1, 2019 · 6 comments

Comments

@oguzhanunlu
Copy link
Member

We need to run setup command and insert a super API key before we can start using the server. We can improve UX of Iglu Server in docker-compose world by replicating setup command within init.sql.

@chuwy
Copy link
Contributor

chuwy commented Nov 1, 2019

Why do we need to duplicate it and cannot use setup itself? setup exists to create a structure that is compatible with DB access layer and serves as a single source of truth. If we change it and forget to reflect it in the init.sql - user will get a corrupted DB, which much worse than not having it at all.

@oguzhanunlu
Copy link
Member Author

I wanted to reduce number of steps for users but I see the corruption point, I'll revert my last change.

Speaking of setup, let me question the existence of setup command. Why does a user need to run one more step when we can automate it by creating tables at server launch if postgres is configured correctly? Isn't it an additional load on user?

@oguzhanunlu oguzhanunlu removed this from the Version 1.0.0 milestone Nov 1, 2019
@chuwy
Copy link
Contributor

chuwy commented Nov 1, 2019

I think these are two semantically different tasks: launching something and deploying infrastructure for something. I personally would not expect something to deploy entities when I intend to launch it. Another questions that we want to raise - how many things we want to auto-deploy (api keys, meta schemas, empty draft etc)? At what point it will become completely opaque to user what they have just deployed during first launch? What happens if at some point we decide to add a non-idempotent entity? Also, how much manual labor does it add to execute one command?

As a middle-ground however there's this ticket: #10

However, I think it makes sense to turn the error message for non-existing table into a guide on what to do.

@oguzhanunlu
Copy link
Member Author

I agree with the idea that these tasks have different semantics while I personally would like to see auto deployment of tables since it isn't usage or user specific and same thing for all usages. I'd not auto-deploy super api key for security concerns.

Even though this step isn't much manual labor, this approach can accumulate itself and result in a future version of this project where each semantically different task requires another command to run. I'm not sure how many different semantics we can possible have here but a point to think about.

I liked #10 . It really looks like a middle ground for this discussion.

From the original ticket:

--with-setup will make server not just running, but also setup the DB tables if they don't exist yet

I think there is another option here; drop tables if exists and create them from scratch. I don't strongly advocate none of the options but curious to see why you preferred the other way.

@istreeter
Copy link
Contributor

I did some work improving the docker-compose setup. See this branch.

Instead of fixing init.sql (which was @oguzhanunlu's suggestion) I added a setup step to docker-compose which runs the migrations, which I think satisfies @chuwy's comments above.

The remaining problem is the user needs to manually add an api key, by executing some sql in the postrgres container. I added instructions to the readme for how to do this, but it's a bit hacky.

A nicer option is to extend the setup command so it takes an optional --master-api-key command line argument. Then it runs the migrations and inserts the key to the database. This would help the docker-compose setup, but would also be helpful for initialising production setups too.

An alternative solution is to add a new command add-api-key so we don't overload the setup command with more functionality. But I slightly prefer my first suggestion.

wdyt?

@chuwy
Copy link
Contributor

chuwy commented May 21, 2021

I like what you did, @istreeter! And think --master-api-key would improve it even further.

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

No branches or pull requests

3 participants