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

Simple server with database #17

Closed
wants to merge 2 commits into from
Closed

Conversation

dikuchan
Copy link
Contributor

@dikuchan dikuchan commented Jul 27, 2020

Using Actix as web framework and Diesel as ORM for Postgres.

In the current state, building procces is complicated and not automated. To successfuly build the server, reproduce next steps (would be useful for @FortyWays):

  1. Install cargo via rustup (using official site, distro package, Docker or whatever).
  2. Install postgresql-devel (name depends on your distro) package in order for Diesel to link during linkage.
  3. Download Docker image for Postgres.
  4. Start a new Postgres instance. For example,
$ docker run -d \
    --name openedx-admin \
    -e POSTGRES_USER=deerc-dev
    -e POSTGRES_PASSWORD=kommsussertod \
    postgres
  1. Set environmental variable DATABASE_URL. For example,
DATABASE_URL=postgres://deerc-dev:[email protected]:5432/openedx-admin

Where username and password are the same as in the previous step, IP could be retrieved from Docker and name of the database is custom. Or, as an alternative, variable can be specified in .env file in the backend directory.
6. Run cargo run in the backend directory. It should successfuly compile and print that it started n workers and a service on localhost. For example,

[2020-07-27T21:43:17Z INFO  actix_server::builder] Starting 4 workers
[2020-07-27T21:43:17Z INFO  actix_server::builder] Starting "actix-web-service-127.0.0.1:8080" service on 127.0.0.1:8080
  1. Check via Postman if request on localhost:8080/hello has status 200.

Additionaly, you can fill the database with sample data. View table structure with DataGrid, for example, then add a new field. JSON response should return all fields in the table.

@dikuchan dikuchan requested review from veotani and FortyWays July 27, 2020 22:11
This was linked to issues Jul 27, 2020
@dikuchan
Copy link
Contributor Author

dikuchan commented Jul 27, 2020

I forgot to mention that the server was built on ClearLinux. I don't guarantee that these steps would work nor on other OSs, neither on other distributions.

@veotani
Copy link
Contributor

veotani commented Jul 28, 2020

@dikuchan I gave it a chance, but the "hello" wasn't delivered to me.
A few things to begin with:

  1. You skipped Simple server #3 and now I have to read code to understand, whether my problem is in Rust or in Rust's connection to DB.
  2. Before you start server, you have to create database. It wasn't mentioned in your PR. You are also resolving Database #7 and this PR is far from it. Look at the description of Database #7. I would rather consider it to be an issue for our DevOps (@N-ihad).
  3. There is no DAL. I expected to see a package for it, but you have DAL related things in API package.
  4. It's better to discuss, what database we are going to pick, before you force PostgreSQL.

Also minor things:

  1. DATABASE_URL=postgres://deerc-dev:[email protected]:5432/openedx-admin: you could use localhost, if you forwarded ports when creating container: docker run -d --name openedx-admin -e POSTGRES_USER=deerc-dev -e POSTGRES_PASSWORD=kommsussertod -p 5432:5432 postgres so that then connection string may be simpler: DATABASE_URL=postgres://deerc-dev:kommsussertod@localhost:5432/openedx-admin.
  2. @FortyWays is not this project member, you probably meant @N-ihad

My suggestions are:

  1. We close this PR and you try to close only Simple server #3
  2. @N-ihad joins and tries to complete Database #7, @dikuchan have to fix problems I wrote above

Please discuss.

use diesel::RunQueryDsl;
use actix_web::{web, HttpResponse};

fn get_users(pool: web::Data<Pool>) -> Result<Vec<User>, diesel::result::Error> {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is DAL method, it shouldn't be in API

web, App, HttpServer,
middleware::Logger,
};
use env_logger::Env;
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it mean we already have loggin? If so, it should be documented somewhere.

.route("/hello", web::get().to(api::auth::hello))
.wrap(Logger::default())
})
.bind("localhost:8080")?
Copy link
Contributor

Choose a reason for hiding this comment

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

Should it be in config or not?

Ok(items)
}

pub async fn hello(db: web::Data<Pool>) -> Result<HttpResponse, actix_web::Error> {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would also like it to be in another package, not in auth. But it's up to you.

@dikuchan dikuchan closed this Jul 28, 2020
@dikuchan dikuchan mentioned this pull request Jul 28, 2020
@dikuchan dikuchan removed a link to an issue Jul 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Database
2 participants