search

Cloud-CV / evalai-cli

:cloud: :rocket: Official EvalAI Command Line Tool
https://cli.eval.ai
BSD 3-Clause "New" or "Revised" License
55 stars 63 forks source link

Remove replication of code to get_token command. #223

Open pushkalkatara opened 4 years ago

pushkalkatara commented 4 years ago

There are two separate blocks of code which perform the same task -

  1. https://github.com/Cloud-CV/evalai-cli/blob/master/evalai/utils/auth.py#L53

  2. https://github.com/Cloud-CV/evalai-cli/blob/master/evalai/get_token.py

Remove replication of code.

daedaldan commented 4 years ago

Since these two blocks of code are essentially identical, would it be feasible to delete get_token.py and replace any usages of that file's code with the identical function in the utils/auth.py file?

m-sameer commented 4 years ago

@pushkalkatara What should we do?

I am thinking that @daniel22373 is right.

pushkalkatara commented 4 years ago

@daniel22373 @m-sameer The task is to use 2 as the wrapper for 1. Rename the file get_token.py to token.py and use the utils to write a click wrapper for get_token function. Just as it is done for other commands here.

add_token would also be integrated in the same file - token.py as it is a token/auth related command, but this would be another issue and another GCI task.

daedaldan commented 4 years ago

Thanks! Could I take this issue then?

On Tuesday, December 31, 2019, Pushkal Katara notifications@github.com wrote:

@daniel22373 https://github.com/daniel22373 @m-sameer https://github.com/m-sameer The task is to use 2 as the wrapper for 1. Just as it is done for other commands here https://github.com/Cloud-CV/evalai-cli/blob/master/evalai/challenges.py.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Cloud-CV/evalai-cli/issues/223?email_source=notifications&email_token=AD2IKJC7JCPUWAYYN2QAIRLQ3O3TPA5CNFSM4J6KEAL2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEH4W3KA#issuecomment-569994664, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD2IKJG5D5REFWP5NOWEMWDQ3O3TPANCNFSM4J6KEALQ .

daedaldan commented 4 years ago

Actually, I have already reached the limit for the number of beginner tasks I can claim on GCI. If anybody else would like to take this issue, that is fine.

On Tuesday, December 31, 2019, Daniel Wang daniel22373@gmail.com wrote:

Thanks! Could I take this issue then?

On Tuesday, December 31, 2019, Pushkal Katara notifications@github.com wrote:

@daniel22373 https://github.com/daniel22373 @m-sameer https://github.com/m-sameer The task is to use 2 as the wrapper for 1. Just as it is done for other commands here https://github.com/Cloud-CV/evalai-cli/blob/master/evalai/challenges.py .

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Cloud-CV/evalai-cli/issues/223?email_source=notifications&email_token=AD2IKJC7JCPUWAYYN2QAIRLQ3O3TPA5CNFSM4J6KEAL2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEH4W3KA#issuecomment-569994664, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD2IKJG5D5REFWP5NOWEMWDQ3O3TPANCNFSM4J6KEALQ .

pushkalkatara commented 4 years ago

@m-sameer Click defines decorators for each function to be used as a CLI command. In order to seperate click decorators and implementation of the function, we have two files token.py and utils/auth.py. In token.py we basically write the click decorators, import the utils/auth.py functions and call the functions directly, which can be called wrapping. In utils/auth.py we have the actual implementation of the function.

m-sameer commented 4 years ago

So I have to create a function get_token in auth.py and point it to the function in token.py?

pushkalkatara commented 4 years ago
  1. Rename get_token.py -> auth.py
  2. Import utils/auth.py in auth.py
  3. update get_token function to use functions present in utils/auth.py
  4. Check if the function works well
  5. Post screenshots of the working command.
m-sameer commented 4 years ago

But you said rename get_token.py to token.py earlier?

pushkalkatara commented 4 years ago

Yes, you can choose either of them.