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

Improved CLI support to pass API key & Device Id #62

Open
rajula96reddy opened this issue Jul 4, 2018 · 10 comments
Open

Improved CLI support to pass API key & Device Id #62

rajula96reddy opened this issue Jul 4, 2018 · 10 comments
Labels
gssoc GirlScript Summer of Code Intermediate

Comments

@rajula96reddy
Copy link

rajula96reddy commented Jul 4, 2018

We need to improve the CLI support for moboff initialise & moboff download in such a way that, we should be able to initialise using moboff initialize --api <key> --device <device_id> or moboff download --api <key> --device <device_id> --link <link>

This will come in handy in #60 & #61

@ashwani99
Copy link
Contributor

I would like to take this up

@Parth-Vader Parth-Vader added Intermediate gssoc GirlScript Summer of Code labels Jul 6, 2018
@ashwani99
Copy link
Contributor

I think the API key can be exported as an environment variable rather than providing it every time with moboff initialise or moboff download
Also as --newdevice is already there, there might not be any need for --device
What do you think? @rajula96reddy

@Parth-Vader
Copy link
Owner

@ashwani99 What are you saying exactly? Do you want to remove moboff initialise completely?

@ashwani99
Copy link
Contributor

ashwani99 commented Jul 8, 2018

@Parth-Vader No, moboff initialise will be there. I mean to say that moboff can read the API key from the environment variable. So the task of moboff initialise becomes only to set the preferred device and path to download. The benefit of this will be that users will not have to run moboff initialise again if they want to change just the API key. Also, there is no need of --device option as --newdevice is already supported.

@rajula96reddy
Copy link
Author

rajula96reddy commented Jul 10, 2018

@ashwani99 @Parth-Vader Sorry, I have been busy over the weekend and I couldn't participate in the discussion. @ashwani99 I think we should either have support to send the API & device through moboff initialise or pass both of them as environmental variables. But if you think about it, let's say we passed the API as Env variable and initialise with a default device say 0, the moboff initialise is interactive and we need to enter the Env variables only when it is prompted, which AFAIK is not easy. Which brings us to the question of 'Can we pass them as arguments?', which as you know is not currently there and needs to be implemented. Also, --newdevice is not an argument for initialise and can only be used with download given we have initialised .

@ashwani99
Copy link
Contributor

ashwani99 commented Jul 10, 2018

But if you think about it, let's say we passed the API as Env variable and initialise with a default device say 0, the moboff initialise is interactive and we need to enter the Env variables only when it is prompted, which AFAIK is not easy.

@rajula96reddy I think you misunderstood me here. I meant the user can export the api key as $ export PUSHBULLET_API_KEY=<key>. Now the program can read the api key from the environment value itself, it doesn't need to ask everytime moboff initialise is ran. Coming to moboff initialise it will display the availalbe devices and will ask us to set default device and also the download path.

The flow will be something like this:

# setting API key
$ export PUSHBULLET_API_KEY=<key>

# choosing device
$ moboff initialise
1 : device 1
2 : device 2
2
Please Select a directory for store the Downloads
The music would be downloaded to <chosen-path>
Now you can run `moboff download` :)

# setting a new default device
$ moboff download --newdevice 1 <link>

@rajula96reddy
Copy link
Author

@ashwani99 Sorry! My bad. Yes! This is a good idea. I think we might have to change the core parts of the code, so I would say let's ask @Parth-Vader's opinion on this. @Parth-Vader can @ashwani99 go ahead make these changes?

@Parth-Vader
Copy link
Owner

@ashwani99 I will tell you why I prefer how the current moboff initialise.

You set the API key (which is meant to not change a lot), and it gets stored in a config file. It's the same as saving it in the environment value since the process of updating both is the same.

It's also a good idea to set a default device, but as an additional feature, the user can provide a new device if he wants to: this is also supposed to be rare for the user.

I agreed that moboff initialise --api <key> --device DEVICE could also be added, but I would prefer it to still be interactive. (No average user wants to pass options like that).

Do note that the name initialise was chosen because it is required to run only once.

@ashwani99
Copy link
Contributor

@Parth-Vader Thanks for the use-case overview. Do you still want --api and --device options to be added? If not, please consider closing this issue.

@Parth-Vader
Copy link
Owner

I would prefer closing this @ashwani99 @rajula96reddy
Are you guys convinced?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gssoc GirlScript Summer of Code Intermediate
Projects
None yet
Development

No branches or pull requests

3 participants