achillean / shodan-python

The official Python library for Shodan
https://developer.shodan.io
Other
2.47k stars 553 forks source link

move config dir to ~/.config #136

Closed Soneji closed 4 years ago

Soneji commented 4 years ago

In reference to #135 I hope this fix is ok with you guys. I notice you didn't have any logic code in settings.py, but I feel as though this is the easiest way to implement this. This method allows users who create the ~/.config/shodan directory to utilise it, while still providing support for the users who do not care to move the folder.

I also ran tox and got

python: commands succeeded
  congratulations :)

Not sure if that's how to run the tests

ghost commented 4 years ago

For new setups, the old ~/.shodan is still used. Intentionally?

Soneji commented 4 years ago

I am by all means happy to set ~/.config/shodan to the default for new installations. As long as that’s OK with the you/the maintainers. The reason behind keeping ~/.shodan Was that I thought the maintainers wanted to keep it there, and only users who care about clogging their home dir with dotfiles would care enough to change it.

ghost commented 4 years ago

I am by all means happy to set ~/.config/shodan to the default for new installations. As long as that’s OK with the you/the maintainers.

I'm not a maintainer of that project :) Nevertheless I agree with you that the ~/.config//$XDG_CONFIG_DIR is the better/correct place

achillean commented 4 years ago

I agree that we should use the correct/ standardized place to store the settings file instead of our own thing for new users. However, do we need to update shodan/main.py:213 to use os.makedirs(shodan_dir) instead of os.mkdir(shodan_dir)? Just in case the .config directory doesn't exist on the machine?

Soneji commented 4 years ago

Good catch! 😉 Forgot about Windows users who may not by default have a ~/.config dir

ghost commented 4 years ago

Good catch! wink Forgot about Windows users who may not by default have a ~/.config dir

Headless systems also might not have that directory I guess.

Should we also (automatically) migrate existing configuration directories to the new location?

achillean commented 4 years ago

If there are other aspects of the library that we'd like to deprecate then I think it would make sense to start migrating before a 2.0 release. Otherwise we'll always need to keep code to check for the existence of .shodan so I'm not sure we're gaining much and introducing a potential new source for errors/ discrepancies. I like that the current solution is simple and does the right thing moving forward.

On Wed, Jun 24, 2020 at 2:29 PM Wagner notifications@github.com wrote:

Good catch! wink Forgot about Windows users who may not by default have a ~/.config dir

Headless systems also might not have that directory I guess.

Should we also (automatically) migrate existing configuration directories to the new location?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/achillean/shodan-python/pull/136#issuecomment-649021512, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAC3I2CFHBEG2FRX7YEYJ6LRYJHYZANCNFSM4OGD7BHA .

-- Shodan - http://www.shodanhq.com

Follow me on Twitter http://twitter.com/achillean

Soneji commented 4 years ago

Yeah I think for this version, migrating existing ~/.shodan dirs to ~/.config/shodan seems pretty problematic. Then the question is, for how long do you need to keep code in the project solely responsible for migrating users' ~/.shodan dirs for?

If you guys are going to release a 2.0 soon, you can just start with using ~/.config/shodan from the new release.

achillean commented 4 years ago

There are no plans for a 2.0 release at the moment as there aren't any major or breaking changes planned. But this would be one of the things that we would want to break with a 2.0 release.

On Wed, Jun 24, 2020 at 4:38 PM Dhaval Soneji notifications@github.com wrote:

Yeah I think for this version, migrating existing ~/.shodan dirs to ~/.config/shodan seems pretty problematic. Then the question is, for how long do you need to keep code in the project solely responsible for migrating users' ~/.shodan dirs for?

If you guys are going to release a 2.0 soon, you can just start with using ~/.config/shodan from the new release.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/achillean/shodan-python/pull/136#issuecomment-649086811, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAC3I2HNOG3CN7J2VEMMZLDRYJW47ANCNFSM4OGD7BHA .

-- Shodan - http://www.shodanhq.com

Follow me on Twitter http://twitter.com/achillean

Soneji commented 4 years ago

Hi,

Any update on the merge?

Soneji commented 4 years ago

@achillean ?