Remove store #713
Merged
Remove store #713
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| App: app, | ||
| Vars: make(Vars), | ||
| }) | ||
| // Return an empty config. |
There was a problem hiding this comment.
This actually exposed a problem with using created_at to determine what is the current config for an app. There was a case where, when setting config on an app that didn't have any releases, 2 configs would get created at about the same time; 1 empty config and 1 with the update vars. Returning an empty config here fixes that race, but using an auto incremented id to order would probably be better.
| if c == nil { | ||
| return p, s.Scheduler.Scale(ctx, release.AppID, string(p.Type), uint(quantity)) | ||
| } else { | ||
| return p, s.releaser.Release(ctx, release) |
There was a problem hiding this comment.
This also fixes the issue where scaling with a new process size (e.g. emp scale web=2:256:20m) would require a new deployment to take effect.
|
👍 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This removes the internal
storetype, in favor of passing around a*gorm.DBinstance. This is better, because it allows us to define better transactional boundaries, so we can wrap methods on the Empire struct in a transaction. This fixes that bug (thought we had an issue for it but couldn't find it) where deploying a docker image that didn't exist would still create an app within Empire, as well as some race conditions during release creation.