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

introduce ability to use custom env_file format #690

Merged
merged 1 commit into from
Oct 7, 2024

Conversation

ndeloof
Copy link
Collaborator

@ndeloof ndeloof commented Oct 1, 2024

This allows to register additional env_file format parser, so one can plug https://github.com/docker/cli/blob/master/opts/envfile.go as an alternate parser, to support reading "raw" env_file just like docker run --env-file does.

@ndeloof ndeloof force-pushed the env_format branch 5 times, most recently from 36e1834 to 2d728e7 Compare October 2, 2024 06:55
@ndeloof ndeloof force-pushed the env_format branch 4 times, most recently from cdb140d to 2e66a7c Compare October 2, 2024 14:34
Copy link
Collaborator

@jhrotko jhrotko left a comment

Choose a reason for hiding this comment

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

LGTM! I would also add/change a test in loader to also cover this case

types/project.go Outdated
fileVars, err = dotenv.ParseWithLookup(file, resolve)
}
if err != nil {
return nil, nil
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
return nil, nil
return nil, err

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

indeed 😓

@ndeloof
Copy link
Collaborator Author

ndeloof commented Oct 2, 2024

@jhrotko a test in loader would make sense if we had an actual alternate format implementation hosted in this repo, but here we just define an extension point. raw loader test is here

Copy link
Collaborator

@jhrotko jhrotko left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Collaborator

@glours glours left a comment

Choose a reason for hiding this comment

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

LGTM

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
@ndeloof ndeloof enabled auto-merge (rebase) October 7, 2024 07:14
@ndeloof ndeloof merged commit 49efa50 into compose-spec:main Oct 7, 2024
8 checks passed
@ndeloof ndeloof deleted the env_format branch October 7, 2024 07:24
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.

3 participants