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

Remove pop from Proxy & RDBMS storage getter #511

Merged
merged 33 commits into from
Aug 21, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
57314df
Remove pop from models for now
manugupt1 Aug 17, 2018
9f09cd7
Remove rdbms storage from pkg/storage
manugupt1 Aug 17, 2018
1777208
Completely remove pop
manugupt1 Aug 17, 2018
e330327
Some improvements to tests
arschles Aug 15, 2018
49b8e6d
update to point to test instance
manugupt1 Aug 17, 2018
6cef1fc
Update databasy.yml for travis??
manugupt1 Aug 17, 2018
2c59093
Merge remote-tracking branch 'origin/remove-rdbms' into remove-pop-rdbms
manugupt1 Aug 17, 2018
1dc9123
Remove database.yml
manugupt1 Aug 17, 2018
0928c47
Do not use db migrations and creation
manugupt1 Aug 17, 2018
fe9e55c
Remove pop dependency from cdn
manugupt1 Aug 17, 2018
6d3098a
add schema for olympus assuming cdn driver is part of olympus
manugupt1 Aug 18, 2018
6dd18fb
Move CI/CD and test scripts to point pop to olympus
manugupt1 Aug 18, 2018
aec759d
Update database.yml and keep it only for olympus
manugupt1 Aug 18, 2018
f1f01fc
Add migrations to travis
manugupt1 Aug 18, 2018
384eaea
Add debug logs to see what travis is doing
manugupt1 Aug 18, 2018
e2046f1
Update travis to use olympus database.yml
manugupt1 Aug 18, 2018
414f4ac
Moarrrr logs to see where travis is pointint the db towards
manugupt1 Aug 18, 2018
aa47460
Change env to test
manugupt1 Aug 18, 2018
c203fed
Use test as the default environment if not specified otherwise
manugupt1 Aug 18, 2018
3884e17
Check if test new rdbms storage succeeds
manugupt1 Aug 18, 2018
1054f41
Try to see which connection string rdbms test suite is using on travis
manugupt1 Aug 18, 2018
0023831
Update db tags for cdn driver
manugupt1 Aug 18, 2018
4349ae2
create db by using travis go_env
manugupt1 Aug 18, 2018
4e036c8
Remove sql from database config as it is removed from docker-compose
manugupt1 Aug 18, 2018
6289fed
Remove extra logs that I missed
manugupt1 Aug 18, 2018
2974ae8
Merge branch 'master' into remove-pop-rdbms
manugupt1 Aug 18, 2018
1f57cdb
Add newline
manugupt1 Aug 18, 2018
3add1e4
Merge branch 'master' into remove-pop-rdbms
manugupt1 Aug 20, 2018
04a471e
Merge branch 'master' into remove-pop-rdbms
manugupt1 Aug 20, 2018
9e69dfc
Merge branch 'master' into remove-pop-rdbms
arschles Aug 20, 2018
c1921a1
remove models.go and test file from proxy
manugupt1 Aug 20, 2018
bdf8211
removing extraneous network prune from Makefile
manugupt1 Aug 20, 2018
97ea321
Merge remote-tracking branch 'upstream/master' into remove-pop-rdbms
manugupt1 Aug 21, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ go:
env:
global:
- PATH=${PATH}:./bin
- POP_PATH=$PWD/cmd/proxy
- GO_ENV=test_postgres
- POP_PATH=$PWD/cmd/olympus
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need POP_PATH at all?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes! because we still have the CDN rdbms driver which is dependent on Pop. So, I made sure that the proxy is independent of Pop but olympus is not.
Once, we have voted on CDN to be kicked out of olympus, the next step would be to create a PR that basically removes POP_PATH from travis

Copy link
Contributor

Choose a reason for hiding this comment

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

@manugupt1 so say we keep CDN, do we still need RDBMS in the CDN? can we not remove RDBMS and use another storage for CDN?

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right on!

So, if we decide not to support the RDBMS driver for CDN, we do not need it at all.
For example,
https://github.com/gomods/athens/blob/master/pkg/cdn/metadata/cdn_metadata_entry.go#L14-L19

Supports both mongo and SQL based tags!
If we remove the rdbms driver that stores the metadata https://github.com/gomods/athens/tree/master/pkg/cdn/metadata/rdbms

Then, we can remove all the dependency.

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe we decided to remove RDBMS from anything Athens related, but we can do it in another PR. @arschles correct me if im wrong :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh cool! I was not aware of of that! Let us wait for @arschles to confirm.

Copy link
Member

Choose a reason for hiding this comment

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

I do believe that RDBMS in CDN is without use. there are no relationships there so any document db will do just fine. Let's get rid of it

Copy link
Member

Choose a reason for hiding this comment

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

Feel free to address it in this or a separate PR @manugupt1

- GO_ENV=test
- MINIO_ACCESS_KEY=minio
- MINIO_SECRET_KEY=minio123
- ATHENS_MONGO_CONNECTION_STRING=mongodb://127.0.0.1:27017
Expand All @@ -20,8 +20,9 @@ before_script:
- make setup-dev-env
- wget "https://dl.minio.io/server/minio/release/linux-amd64/minio"
- chmod +x minio && nohup ./minio server . &
- buffalo db create
- buffalo db migrate up
- buffalo db create -e $GO_ENV -d -c $POP_PATH/database.yml
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need this if we're removing pop and rdbms storage?
Same thing for the line below.

Copy link
Member Author

Choose a reason for hiding this comment

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

See below!

- buffalo db migrate up -e $GO_ENV -d -c $POP_PATH/database.yml


script:
- make verify test-unit test-e2e
Expand Down
1 change: 0 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ bench:

.PHONY: alldeps
alldeps:
docker-compose -p athensdev up -d mysql
docker-compose -p athensdev up -d postgres
docker-compose -p athensdev up -d mongo
docker-compose -p athensdev up -d redis
Expand Down
5 changes: 0 additions & 5 deletions cmd/olympus/actions/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,6 @@ func App(config *AppConfig) (*buffalo.App, error) {
// TODO: initialize the azurecdn.Storage struct here
}))

// Wraps each request in a transaction.
// c.Value("tx").(*pop.PopTransaction)
// Remove to disable this.
// app.Use(middleware.PopTransaction(models.DB))

// Setup and use translations:
if T, err = i18n.New(packr.NewBox("../locales"), "en-US"); err != nil {
app.Stop(err)
Expand Down
7 changes: 0 additions & 7 deletions cmd/olympus/actions/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"github.com/gomods/athens/pkg/storage/fs"
"github.com/gomods/athens/pkg/storage/mem"
"github.com/gomods/athens/pkg/storage/mongo"
"github.com/gomods/athens/pkg/storage/rdbms"
"github.com/spf13/afero"
)

Expand All @@ -35,12 +34,6 @@ func GetStorage() (storage.Backend, error) {
}
certPath := env.MongoCertPath()
return mongo.NewStorageWithCert(connectionString, certPath)
case "postgres", "sqlite", "cockroach", "mysql":
connectionName, err := env.RdbmsName()
if err != nil {
return nil, err
}
return rdbms.NewRDBMSStorage(connectionName)
default:
return nil, fmt.Errorf("storage type %s is unknown", storageType)
}
Expand Down
25 changes: 9 additions & 16 deletions cmd/olympus/database.yml
Original file line number Diff line number Diff line change
@@ -1,29 +1,22 @@
development:
dialect: "mysql"
dialect: "postgres"
database: athens
host: 127.0.0.1
port: 3306
user: vgp
password: vgp
port: 5432
user: postgres
password: postgres

test_postgres:
test:
dialect: "postgres"
database: athens_development
database: athens_test
user: postgres
password: ''
password: postgres
port: 5432
host: 127.0.0.1
pool: 5

test:
dialect: "mysql"
database: athens
host: 127.0.0.1
port: 3306
user: vgp
password: vgp

production:
dialect: "mysql"
dialect: "postgres"
database: olympusdb
host: {{ env "DB_HOST" }}
port: {{ env "DB_PORT" }}
Expand Down
97 changes: 57 additions & 40 deletions cmd/olympus/migrations/schema.sql
Original file line number Diff line number Diff line change
@@ -1,40 +1,57 @@
-- MySQL dump 10.13 Distrib 5.7.21, for osx10.13 (x86_64)
--
-- Host: 127.0.0.1 Database: athens
-- ------------------------------------------------------
-- Server version 5.7.21

/*!40101 SET @OLD_CHARACTER_SET_CLIENT=@@CHARACTER_SET_CLIENT */;
/*!40101 SET @OLD_CHARACTER_SET_RESULTS=@@CHARACTER_SET_RESULTS */;
/*!40101 SET @OLD_COLLATION_CONNECTION=@@COLLATION_CONNECTION */;
/*!40101 SET NAMES utf8 */;
/*!40103 SET @OLD_TIME_ZONE=@@TIME_ZONE */;
/*!40103 SET TIME_ZONE='+00:00' */;
/*!40014 SET @OLD_UNIQUE_CHECKS=@@UNIQUE_CHECKS, UNIQUE_CHECKS=0 */;
/*!40014 SET @OLD_FOREIGN_KEY_CHECKS=@@FOREIGN_KEY_CHECKS, FOREIGN_KEY_CHECKS=0 */;
/*!40101 SET @OLD_SQL_MODE=@@SQL_MODE, SQL_MODE='NO_AUTO_VALUE_ON_ZERO' */;
/*!40111 SET @OLD_SQL_NOTES=@@SQL_NOTES, SQL_NOTES=0 */;

--
-- Table structure for table `schema_migration`
--

DROP TABLE IF EXISTS `schema_migration`;
/*!40101 SET @saved_cs_client = @@character_set_client */;
/*!40101 SET character_set_client = utf8 */;
CREATE TABLE `schema_migration` (
`version` varchar(255) NOT NULL,
UNIQUE KEY `version_idx` (`version`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8;
/*!40101 SET character_set_client = @saved_cs_client */;
/*!40103 SET TIME_ZONE=@OLD_TIME_ZONE */;

/*!40101 SET SQL_MODE=@OLD_SQL_MODE */;
/*!40014 SET FOREIGN_KEY_CHECKS=@OLD_FOREIGN_KEY_CHECKS */;
/*!40014 SET UNIQUE_CHECKS=@OLD_UNIQUE_CHECKS */;
/*!40101 SET CHARACTER_SET_CLIENT=@OLD_CHARACTER_SET_CLIENT */;
/*!40101 SET CHARACTER_SET_RESULTS=@OLD_CHARACTER_SET_RESULTS */;
/*!40101 SET COLLATION_CONNECTION=@OLD_COLLATION_CONNECTION */;
/*!40111 SET SQL_NOTES=@OLD_SQL_NOTES */;

-- Dump completed on 2018-06-06 12:42:59
--
-- PostgreSQL database dump
--

-- Dumped from database version 9.6.9
-- Dumped by pg_dump version 10.4

SET statement_timeout = 0;
SET lock_timeout = 0;
SET idle_in_transaction_session_timeout = 0;
SET client_encoding = 'UTF8';
SET standard_conforming_strings = on;
SELECT pg_catalog.set_config('search_path', '', false);
SET check_function_bodies = false;
SET client_min_messages = warning;
SET row_security = off;

--
-- Name: plpgsql; Type: EXTENSION; Schema: -; Owner:
--

CREATE EXTENSION IF NOT EXISTS plpgsql WITH SCHEMA pg_catalog;


--
-- Name: EXTENSION plpgsql; Type: COMMENT; Schema: -; Owner:
--

COMMENT ON EXTENSION plpgsql IS 'PL/pgSQL procedural language';


SET default_tablespace = '';

SET default_with_oids = false;

--
-- Name: schema_migration; Type: TABLE; Schema: public; Owner: postgres
--

CREATE TABLE public.schema_migration (
version character varying(255) NOT NULL
);


ALTER TABLE public.schema_migration OWNER TO postgres;

--
-- Name: schema_migration_version_idx; Type: INDEX; Schema: public; Owner: postgres
--

CREATE UNIQUE INDEX schema_migration_version_idx ON public.schema_migration USING btree (version);


--
-- PostgreSQL database dump complete
--

19 changes: 1 addition & 18 deletions cmd/olympus/models/models.go
Original file line number Diff line number Diff line change
@@ -1,22 +1,5 @@
package models

import (
"log"

"github.com/gobuffalo/pop"
"github.com/gomods/athens/pkg/config/env"
)

// DB is a connection to your database to be used
// throughout your application.
var DB *pop.Connection

func init() {
var err error
env := env.GoEnvironmentWithDefault("development")
DB, err = pop.Connect(env)
if err != nil {
log.Fatal(err)
}
pop.Debug = env == "development"

}
9 changes: 1 addition & 8 deletions cmd/olympus/models/models_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,8 @@ package models_test

import (
"testing"

"github.com/gobuffalo/suite"
)

type ModelSuite struct {
*suite.Model
}

func Test_ModelSuite(t *testing.T) {
as := &ModelSuite{suite.NewModel()}
suite.Run(t, as)

}
6 changes: 0 additions & 6 deletions cmd/proxy/actions/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,12 +119,6 @@ func App() (*buffalo.App, error) {
csrfMiddleware := csrf.New
app.Use(csrfMiddleware)
}

// Wraps each request in a transaction.
// c.Value("tx").(*pop.PopTransaction)
// Remove to disable this.
// app.Use(middleware.PopTransaction(models.DB))

// Setup and use translations:
if T, err = i18n.New(packr.NewBox("../locales"), "en-US"); err != nil {
app.Stop(err)
Expand Down
7 changes: 0 additions & 7 deletions cmd/proxy/actions/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"github.com/gomods/athens/pkg/storage/mem"
"github.com/gomods/athens/pkg/storage/minio"
"github.com/gomods/athens/pkg/storage/mongo"
"github.com/gomods/athens/pkg/storage/rdbms"
"github.com/spf13/afero"
)

Expand Down Expand Up @@ -43,12 +42,6 @@ func GetStorage() (storage.Backend, error) {
return nil, fmt.Errorf("could not create new storage from os fs (%s)", err)
}
return s, nil
case "postgres", "sqlite", "cockroach", "mysql":
storageRoot, err = env.RdbmsName()
if err != nil {
return nil, err
}
return rdbms.NewRDBMSStorage(storageRoot)
case "minio":
endpoint, err := env.MinioEndpoint()
if err != nil {
Expand Down
31 changes: 0 additions & 31 deletions cmd/proxy/database.yml

This file was deleted.

22 changes: 0 additions & 22 deletions cmd/proxy/models/models.go

This file was deleted.

16 changes: 0 additions & 16 deletions cmd/proxy/models/models_test.go

This file was deleted.

12 changes: 3 additions & 9 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,13 @@ services:
image: mongo:3.7.9-jessie
ports:
- 27017:27017
mysql:
image: bitnami/mysql:5.7.21-r7
ports:
- "3306:3306"
environment:
- "ALLOW_EMPTY_PASSWORD=yes"
- "MYSQL_USER=vgp"
- "MYSQL_PASSWORD=vgp"
- "MYSQL_DATABASE=athens"
postgres:
image: postgres:9.6.9-alpine
ports:
- "5432:5432"
environment:
POSTGRES_USER: postgres
POSTGRES_PASSWORD: postgres
minio:
image: minio/minio:latest
command: server /data
Expand Down
Loading