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.

Sign up for GitHub

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

Jump to bottom

Add dotenv support #2

Merged
merged 3 commits into from
May 16, 2023
Merged

Add dotenv support #2

merged 3 commits into from
May 16, 2023

Conversation

zerolab
Copy link
Collaborator

@zerolab zerolab commented May 15, 2023

This PR adds python-dotenv as a dependency to support .env files as I consistently fail to run export ...

tomdyson
tomdyson requested changes May 16, 2023
View reviewed changes
Copy link
Owner

@tomdyson tomdyson left a comment

Choose a reason for hiding this comment

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

Looks great, thanks! Apart from the .gitignore thing - would be good to get that confirmed.

.gitignore Outdated

cache/
index.html
Copy link
Owner

Choose a reason for hiding this comment

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

Unfortunately we can't .gitignore index.html and prompt.txt because Google Cloud Run doesn't upload files which are matched by .gitignore. At least that's my experience - I can't find a reference to this behaviour!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

TIL! will amend

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

https://cloud.google.com/sdk/gcloud/reference/topic/gcloudignore mentions

The default content of the generated .gcloudignore file, which can be overriden with --ignore-file, is as follows:

 .gcloudignore
 .git
 .gitignore

...
In order to ignore files specified in the gitignore file, there is a special comment syntax:

#!include:.gitignore

So you are right

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thinking out loud.. given the fact nldb is intended to be installed as a package/dependency, it should not contain this .gitignore 🤔

@tomdyson tomdyson merged commit 4953b19 into tomdyson:main May 16, 2023
@tomdyson
Copy link
Owner

Thanks @zerolab!

@zerolab zerolab deleted the chore/dotenv branch May 16, 2023 22:10
@zerolab zerolab mentioned this pull request May 17, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomdyson tomdyson tomdyson approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@zerolab @tomdyson