Skip to content

Conversation

nahuelhds
Copy link
Contributor

Target: to keep my credentails and tokens secretly so I can use them safely in services like Heroku, Vercel or any other.

Usage

The configuration file

In the config.yaml file

twitter:
  consumer_key: "${MY_CONSUMER_KEY_ENV_VAR}"
  consumer_secret: "${MY_CONSUMER_SECRET_ENV_VAR}"

Defining the env vars

Every service like Heroku, Vercel, etc, offers a way to define environment vars for the project. Just define them there.

Othewsie, you can define them locally (or remotely too if you have your own server), by creating a .env file, this way:

MY_CONSUMER_KEY_ENV_VAR=CONSUMER_KEY
MY_CONSUMER_SECRET_ENV_VAR=CONSUMER_SECRET

Of course, this file has to be ignored in the version control.

```

Done! You can use diffengine as usual and keep your credentials safe.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Feel free to edit this at will

config_file = os.path.join(home, "config.yaml")
if os.path.isfile(config_file):
config = yaml.load(open(config_file), Loader=yaml.FullLoader)
config = EnvYAML(config_file)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here is the actual change.

All the others one are automatic whitespaces replacements that my IDEs does automatically

@edsu
Copy link
Member

edsu commented Apr 26, 2020

Looks good! So it looks like you would rather submit discrete pull requests for new functionality instead of working with the [threads] branch that merged in your entire master? That is fine with me. I just thought it could be easier to shape what you have on master since you have added quite a few new things.

@nahuelhds
Copy link
Contributor Author

Yes, I really prefer this approach so I can make the proper tests and refactor the code properly!

@edsu edsu merged commit 564b26e into DocNow:master Apr 26, 2020
@nahuelhds
Copy link
Contributor Author

I've also created an issue to see if it's possible to define a common formatter for the entire project. So when I pull changes they really only focus on the what actually changed and not about formatting diffs (like happened here with some whitespaces)

@nahuelhds nahuelhds deleted the feature/envyaml branch April 26, 2020 15:53
@edsu
Copy link
Member

edsu commented Apr 26, 2020

After I merged this I noticed that the test is failing, and was wondering if you are seeing the same?

FAILED test_diffengine.py::test_environment_vars_in_config_file - TypeError: home_path() takes 1 posit...
==========
E       TypeError: home_path() takes 1 positional argument but 2 were given

@edsu
Copy link
Member

edsu commented Apr 26, 2020

I'll see if I can fix this, which will help me understand the changes better!

edsu added a commit that referenced this pull request Apr 26, 2020
Some small changes to get this test to work. Unfortunately
diffenigne.config is a global in the diffengine module, and
changing it doesn't play nicely with the tests. I added a
return value for diffengine.load_config that makes this easier.

refs #67
@edsu
Copy link
Member

edsu commented Apr 26, 2020

@nahuelhds make sure you merge the latest from master for this change to get the tests working again.

@edsu
Copy link
Member

edsu commented Apr 26, 2020

Hmm, the test works for me locally, but not in Travis. Maybe Travis already uses .env? Maybe we can skip this test on Travis...

@edsu
Copy link
Member

edsu commented Apr 26, 2020

I marked the test for skipping when running under Travis.

@nahuelhds
Copy link
Contributor Author

Oh you use Travis. Wouldn't be nice to connect Travis to your master repo so we could wait for the CI to pass and avoid this kind of issue, don't you think?

Regard the issue itself, I'm seeing now that fails for me locally but don't worry. I'll try to make it work by using another env file name so it doens't mess with the original one from Travis

@edsu
Copy link
Member

edsu commented Apr 26, 2020

Yes, Travis should only run on the master branch. I just pushed a change for that to Master. I'll be more careful about verifying new tests work before merging in the future!

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.

2 participants