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

refactor admin server desing #117

Merged

Conversation

mahmednabil109
Copy link
Contributor

  • separate storage entities from server entities
  • use proper struct tags

Signed-off-by: Mohamed Abokammer [email protected]

mimir-d
mimir-d previously approved these changes Jul 29, 2022
@@ -56,7 +57,7 @@ func toMongoQuery(query storage.Query) bson.D {

if query.Text != nil {
q = append(q, bson.E{
Key: "logdata",
Key: "log_data",
Copy link
Member

Choose a reason for hiding this comment

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

note that this is a breaking change, youre changing a db field and consumers may already be using the last version of the field names in their mongo deployments

i will approve this this time, as the project state is technically "not launched", but you should consider a migration mechanism for these updates

Copy link
Member

Choose a reason for hiding this comment

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

you might want to consider a db migration/versioning scheme later on to address this

cmds/admin_server/storage/mongo/mongo.go Show resolved Hide resolved
@mahmednabil109 mahmednabil109 force-pushed the Refactor_admin_server_design branch 2 times, most recently from b943f49 to 6c3d05f Compare August 1, 2022 13:41
- separate storage entities from server entities

Signed-off-by: Mohamed Abokammer <[email protected]>
@mahmednabil109 mahmednabil109 force-pushed the Refactor_admin_server_design branch from 6c3d05f to 5e259ce Compare August 1, 2022 13:56
@mimir-d mimir-d merged commit 9fd8de1 into linuxboot:main Aug 1, 2022
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.

2 participants