Skip to content
This repository has been archived by the owner on Apr 3, 2024. It is now read-only.

fix: Ensures directory path to db file exists #140

Merged
merged 2 commits into from
Sep 24, 2022

Conversation

hferentschik
Copy link
Contributor

fixes #122

@CLAassistant
Copy link

CLAassistant commented Sep 23, 2022

CLA assistant check
All committers have signed the CLA.

cmd/temporalite/main.go Outdated Show resolved Hide resolved
cmd/temporalite/main.go Outdated Show resolved Hide resolved
cmd/temporalite/main.go Outdated Show resolved Hide resolved
@hferentschik hferentschik force-pushed the issue-122 branch 2 times, most recently from 945a7ac to c6fb90f Compare September 23, 2022 18:54
Copy link
Member

@cretz cretz left a comment

Choose a reason for hiding this comment

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

Approved, but would like @jlegrone's approval before merging

cmd/temporalite/main.go Outdated Show resolved Hide resolved
cmd/temporalite/main.go Outdated Show resolved Hide resolved
@@ -162,6 +163,13 @@ func buildCLI() *cli.App {
return cli.Exit(fmt.Sprintf("ERROR: only one of %q or %q flags may be passed at a time", ephemeralFlag, dbPathFlag), 1)
}

// Make sure the default db path exists (user does not specify path explicitly)
if !c.IsSet(dbPathFlag) {
Copy link
Member

Choose a reason for hiding this comment

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

Can you confirm this is not considered "set" when left as default?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes this leads to the correct behavior. I'd like to add an integration test though.

@jlegrone jlegrone enabled auto-merge (squash) September 24, 2022 18:39
@jlegrone jlegrone merged commit 8833863 into temporalio:main Sep 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error when following getting started guide
4 participants