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

Add export to sqlite feature #154

Merged
merged 1 commit into from
Aug 31, 2018
Merged

Add export to sqlite feature #154

merged 1 commit into from
Aug 31, 2018

Conversation

zhouzhuojie
Copy link
Collaborator

@zhouzhuojie zhouzhuojie commented Aug 31, 2018

Description

  • Refactor DB related code from repo to handler package
  • Add export/sqlite endpoint for exporting the whole database

Motivation and Context

I want to setup Flagr for environment other than production. For example, the integration testing env, staging env, or just local unit test env. There's no easy way to copy flags from one flagr deploy to another, and we probably don't want to share the Flagr instances, their databases, or reuse the same Flagr polluting data recording.

This PR builds an interface for exporting data to a simple sqlite file, which can be used for other Flagr instances.

I'm also thinking of serialization into some configuration files, like YAML or TOML. For example, export/yaml, and the files can be checked into code for testing or other environment. It may require some work for (de)serialization.

How Has This Been Tested?

Locally. Unit tests coming soon.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@codecov-io
Copy link

codecov-io commented Aug 31, 2018

Codecov Report

Merging #154 into master will increase coverage by 0.16%.
The diff coverage is 95%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #154      +/-   ##
==========================================
+ Coverage   93.16%   93.32%   +0.16%     
==========================================
  Files          24       25       +1     
  Lines        1244     1274      +30     
==========================================
+ Hits         1159     1189      +30     
+ Misses         58       57       -1     
- Partials       27       28       +1
Impacted Files Coverage Δ
pkg/entity/fixture.go 100% <ø> (ø) ⬆️
pkg/mapper/entity_restapi/e2r/e2r.go 100% <ø> (ø) ⬆️
pkg/entity/db.go 80% <100%> (ø)
pkg/handler/eval_cache.go 89.28% <100%> (ø) ⬆️
pkg/config/middleware.go 100% <100%> (ø) ⬆️
pkg/handler/export.go 100% <100%> (ø)
pkg/config/config.go 84.21% <50%> (-4.37%) ⬇️
pkg/handler/handler.go 97.5% <75%> (+5.19%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5a5e61c...f20b66d. Read the comment docs.

Copy link
Member

@marceloboeira marceloboeira left a comment

Choose a reason for hiding this comment

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

Looks good code-wise. (Even though a lot changed 👍 )

Just curious, whats the motivation around this?

@zhouzhuojie zhouzhuojie mentioned this pull request Aug 31, 2018
8 tasks
@zhouzhuojie zhouzhuojie force-pushed the zz/add-sqlite-backup branch from 28c1c77 to f20b66d Compare August 31, 2018 20:30
@@ -9,10 +9,8 @@ var Config = struct {
// Port - Flagr server port
Port int `env:"PORT" envDefault:"18000"`

// GracefulCleanupTimeout - the timeout after graceful shutdown. It's useful to mitigate Kubernetes
// rolling update race condition.
GracefulCleanupTimeout time.Duration `env:"FLAGR_GRACEFUL_CLEANUP_TIMEOUT" envDefault:"2s"`
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not needed as the latest go-swagger supports graceful shutdown

@@ -28,8 +28,12 @@ func init() {
}

func setupLogrus() {
l, err := logrus.ParseLevel(Config.LogrusLevel)
if err != nil {
logrus.WithField("err", err).Fatalf("failed to set logrus level:%s", Config.LogrusLevel)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

respect the env setting for log level

"github.com/checkr/flagr/pkg/util"

"github.com/checkr/flagr/swagger_gen/models"
)

var getDB = repo.GetDB
var getDB = entity.GetDB
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

refactor

@zhouzhuojie zhouzhuojie requested review from ziru and rstratton August 31, 2018 20:36
200:
description: >
Export sqlite3 format of the db dump, which is converted from the
from the main database.

Choose a reason for hiding this comment

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

Duplicate "from the"

'200':
description: >
Export sqlite3 format of the db dump, which is converted from the
from the main database.

Choose a reason for hiding this comment

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

Duplicate "from the"

@zhouzhuojie zhouzhuojie force-pushed the zz/add-sqlite-backup branch from f20b66d to 382769a Compare August 31, 2018 22:03
@zhouzhuojie zhouzhuojie merged commit 1601f0e into master Aug 31, 2018
@zhouzhuojie zhouzhuojie deleted the zz/add-sqlite-backup branch August 31, 2018 22:10
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.

5 participants