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

controller: Move hstore to jsonb #1833

Merged
merged 1 commit into from Sep 6, 2015

Conversation

Projects
None yet
2 participants
@josephglanville
Copy link
Contributor

commented Sep 5, 2015

This removes usage of hstore as it's overly complicated for our usecase and provides negligible performance benefit. It also makes migration to pgx easier as we would have needed to port to pgx hstore API.

@josephglanville josephglanville force-pushed the josephglanville:remove_hstore branch from 7b87f7c to d0741ba Sep 5, 2015

app.Meta[k] = v.String
}
if len(meta) > 0 {
err = json.Unmarshal(meta, &app.Meta)

This comment has been minimized.

Copy link
@titanous

titanous Sep 5, 2015

Member

This nukes the previous error, add a err != nil check above.

d.Processes[k] = n
}
}
err = json.Unmarshal(procs, &d.Processes)

This comment has been minimized.

Copy link
@titanous

titanous Sep 5, 2015

Member

Ditto on the err overwrite.

for k, v := range env.Map {
r.Env[k] = v.String
if len(env) > 0 {
err = json.Unmarshal(env, &r.Env)

This comment has been minimized.

Copy link
@titanous

titanous Sep 5, 2015

Member

Another err bites the dust.

@josephglanville josephglanville force-pushed the josephglanville:remove_hstore branch 2 times, most recently from ff96d4a to 3583189 Sep 5, 2015

@josephglanville

This comment has been minimized.

Copy link
Contributor Author

commented Sep 6, 2015

err clobbering fixed up.

var releaseID *string
err := s.Scan(&app.ID, &app.Name, &meta, &app.Strategy, &releaseID, &app.CreatedAt, &app.UpdatedAt)
if err == sql.ErrNoRows {
err = ErrNotFound
return nil, ErrNotFound

This comment has been minimized.

Copy link
@titanous

titanous Sep 6, 2015

Member

This isn't the only error that can be returned from Scan, probably should add an else if err != nil

@josephglanville josephglanville force-pushed the josephglanville:remove_hstore branch from 3583189 to 4bb8573 Sep 6, 2015

controller: Move hstore to jsonb
Signed-off-by: Joseph Glanville <jpg@jpg.id.au>

@josephglanville josephglanville force-pushed the josephglanville:remove_hstore branch from 4bb8573 to 000c49a Sep 6, 2015

@titanous

This comment has been minimized.

Copy link
Member

commented Sep 6, 2015

LGTM.

titanous added a commit that referenced this pull request Sep 6, 2015

Merge pull request #1833 from josephglanville/remove_hstore
controller: Move hstore to jsonb

@titanous titanous merged commit 44b8db1 into flynn:master Sep 6, 2015

1 check passed

continuous-integration/flynn The Flynn CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.