minrk / findspark

BSD 3-Clause "New" or "Revised" License
511 stars 72 forks source link

Added option to persist changes by editing bash and IPython profiles #3

Closed alope107 closed 9 years ago

alope107 commented 9 years ago

Adds a persist_changes option that appends commands to set the required environment variables to .bashrc and .cshrc so that findspark only needs to be called once.

minrk commented 9 years ago

If we persist it to an IPython startup file, it won't be shell or env dependent. What do you think about doing that?

alope107 commented 9 years ago

Sounds like a good idea to me. Do you think it's worth having the shell config edits as an option as well, or should I just replace this PR with one that only has an option for the IPython startup?

minrk commented 9 years ago

Good question. The shell variables would be useful for scripts outside IPython, for instance, but I'm not sure how valuable that is. I happen to have a very strong preference against things that touch my bashrc, but the persist option is off by default, so it's probably okay.

WIth all that, I am probably ever-so-slightly against saving to the shell rc, but if someone has a compelling reason to do it, it should be fine.

alope107 commented 9 years ago

I added the option to write to an IPython profile as well. It can create a new profile or append to an existing one. I was unsure whether it would be preferable to call IPython's profile commands through subprocess or directly through the IPython core classes. This commit uses subprocess. Would you favor the other approach?

alope107 commented 9 years ago

Thanks for the feedback. I've incorporated the suggested changes.

alope107 commented 9 years ago

Code now only appends to existing config files. I don't have a particular reason for configuring csh other than supporting more environments.

minrk commented 9 years ago

In that case, I'd probably remove csh until a user explicitly asks for it.

alope107 commented 9 years ago

OK, I'll take it out. Is the reasoning that we don't want to put on the burden of maintaining features that we don't know are being used?

minrk commented 9 years ago

Yup, to some degree.