-
Notifications
You must be signed in to change notification settings - Fork 63
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
Remove replication of code to get_token command. #223
Comments
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? |
@pushkalkatara What should we do? I am thinking that @daniel22373 is right. |
@daniel22373 @m-sameer 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. |
Thanks! Could I take this issue then?
…On Tuesday, December 31, 2019, Pushkal Katara ***@***.***> 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
<#223?email_source=notifications&email_token=AD2IKJC7JCPUWAYYN2QAIRLQ3O3TPA5CNFSM4J6KEAL2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEH4W3KA#issuecomment-569994664>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AD2IKJG5D5REFWP5NOWEMWDQ3O3TPANCNFSM4J6KEALQ>
.
|
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 ***@***.***> wrote:
Thanks! Could I take this issue then?
On Tuesday, December 31, 2019, Pushkal Katara ***@***.***>
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
> <#223?email_source=notifications&email_token=AD2IKJC7JCPUWAYYN2QAIRLQ3O3TPA5CNFSM4J6KEAL2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEH4W3KA#issuecomment-569994664>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AD2IKJG5D5REFWP5NOWEMWDQ3O3TPANCNFSM4J6KEALQ>
> .
>
|
@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. |
So I have to create a function |
|
But you said rename get_token.py to token.py earlier? |
Yes, you can choose either of them. |
There are two separate blocks of code which perform the same task -
https://github.com/Cloud-CV/evalai-cli/blob/master/evalai/utils/auth.py#L53
https://github.com/Cloud-CV/evalai-cli/blob/master/evalai/get_token.py
Remove replication of code.
The text was updated successfully, but these errors were encountered: