wip-flag-migration #28
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: urdt/ussd#28
Reference in New Issue
Block a user
Delete Branch "wip-flag-migration"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Silly, I made the pull request so the review can only be marked as comment...
@@ -54,0 +48,4 @@}// Register all flags loaded from pp.csvflagKeys := []string{these flags should be loaded not hardcoded.
@@ -54,0 +67,4 @@"flag_incorrect_date_format",}for _, key := range flagKeys {iterate on csv lines and populate with rows 2 and 3...
@@ -38,2 +66,3 @@func NewHandlers(path string, st *state.State) *Handlers {func NewHandlers(dir string, st *state.State) (*Handlers, error) {db, err := gdbm.Open(dbFile, gdbm.ModeWrcreat)FYI this will truncate the file on every open.
... but lets not spend time on that, as it needs to be changed to the new gdbm implementation anyway.
So a question on this,we want to be able to keep a reference to the file created for each session and be able to initialize a database instance depending on its existence.So we have this code and it still seems to be removing the previously stored information when it is opened with mode ModeWriter .A fix for the time being for this will be appreciated because the refactoring depends on storing the user information to gdbm.
First, as I said, let's not spend time on this, since this code will be gone when you implement db.gdbmDb in dev-0.1.0.
That said, choosing flag on os.Stat would be the way to go, I guess.
As a reference example on how to do it directly, see
go-visebranchdev-0.1.0methoddb/gdbm/gdbmDb.Connect(...)But I repeat, please don't spend time on this.
@@ -60,1 +98,4 @@func (h *Handlers) PreloadFlags(flagKeys []string) (map[string]uint32, error) {flags := make(map[string]uint32)for _, key := range flagKeys {It shouldn't be necessary to process the flags twice.
@@ -85,2 +143,3 @@err := h.accountFileHandler.EnsureFileExists()// Preload the required flagsflagKeys := []string{"flag_account_created", "flag_account_creation_failed"}also seems unnecessary, there should be one single source of flag label to value map.
For clarity, do you mean that we should load all flags once in the menuhandler and have functions access the already loaded flags?
yes, or perhaps even globally.
@@ -88,3 +148,4 @@return res, err}// err = h.accountFileHandler.EnsureFileExists()please remove commented code if it is not of any more use
@@ -156,3 +222,2 @@case "2":res.FlagReset = append(res.FlagSet, models.USERFLAG_ALLOW_UPDATE)res.FlagSet = append(res.FlagSet, models.USERFLAG_SINGLE_EDIT)res.FlagReset = append(res.FlagSet, flags["flag_allow_update"])here appends to set but assign to reset. happens in the below, too.
@@ -546,1 +629,3 @@return res, errerr = h.db.Delete([]byte(Amount))if err != nil && !errors.Is(err, gdbm.ErrItemNotFound) {panic(err)we cannot have panics anywhere in the vm execution! Just return error.
@@ -54,0 +71,4 @@}// Register the flagstate.FlagDebugger.Register(uint32(flagValue), flagName)A logline to show debug the flag loaded would be nice
@@ -32,0 +56,4 @@// FlagManager handles centralized flag managementtype FlagManager struct {parser *asm.FlagParsermu sync.RWMutexI don't think mutex is needed here, it will be loaded once (before threads) and only read from then right?
@@ -87,0 +110,4 @@ussdHandlers, err := ussd.NewHandlers(fp, &st, sessionId)if err != nil {fmt.Fprintf(os.Stderr, "handler setup failed with error: %v\n", err)Exit in main if any setup fails.