Skip to content

Create instance db#1033

Merged
ilgooz merged 13 commits intodevfrom
feature/instance-data
Jun 16, 2019
Merged

Create instance db#1033
ilgooz merged 13 commits intodevfrom
feature/instance-data

Conversation

@antho1404
Copy link
Copy Markdown
Member

Add instance struct and instance DB

NicolasMahe
NicolasMahe previously approved these changes Jun 7, 2019
Copy link
Copy Markdown
Member

@NicolasMahe NicolasMahe left a comment

Choose a reason for hiding this comment

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

One small typo. Otherwise, ready to merge 👍

Comment thread config/config.go Outdated
@antho1404 antho1404 force-pushed the feature/instance-data branch from 390ad8e to 4321b75 Compare June 7, 2019 10:02
ilgooz
ilgooz previously approved these changes Jun 7, 2019
Comment thread database/instance_db.go Outdated
Comment thread database/instance_db.go Outdated
Comment thread database/instance_db.go
Comment thread instance/instance.go Outdated
krhubert
krhubert previously approved these changes Jun 10, 2019
@ilgooz
Copy link
Copy Markdown
Contributor

ilgooz commented Jun 13, 2019

are you sure that writes are atomic without using transactions? it doesn't worth to rm transactions now since db is going to be changed soon, there is no reason to introduce any instability in this period.

@ilgooz
Copy link
Copy Markdown
Contributor

ilgooz commented Jun 14, 2019

@antho1404 this is blocking, can you give the last decision about transactions please?

@antho1404
Copy link
Copy Markdown
Member Author

I don't see the issue with the transaction, this is already used in the execution db so I don't see why there is an issue with that

@ilgooz ilgooz force-pushed the feature/instance-data branch from 48bd913 to 55c5adb Compare June 15, 2019 16:09
Copy link
Copy Markdown
Contributor

@ilgooz ilgooz left a comment

Choose a reason for hiding this comment

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

I don't approve because it's not clear removing transactions is ok. Tho, I merge anyway since db will be changed in near time.

@ilgooz ilgooz merged commit a749d54 into dev Jun 16, 2019
@ilgooz ilgooz deleted the feature/instance-data branch June 16, 2019 16:00
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.

4 participants