-
Notifications
You must be signed in to change notification settings - Fork 10
Link your codeforces profile with cli #26
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
base: main
Are you sure you want to change the base?
Conversation
|
Kindly provide description for each PR you raise. |
|
By linking I mean that we can store current user info somewhere. Your are able to display some random user as of now. Can we store that so that we can use this info somewhere in other commands. Also, I think that this command should be included in config command group. Kindly put your arguments of creating a new command group of it. |
|
Sir please check now i have created function to save and fetch data about user in config file |
src/Config/Config.py
Outdated
| config["firstname"] = html_content["result"][0]["firstName"] | ||
| config["lastname"] = html_content["result"][0]["lastName"] | ||
| config["rating"] = str(html_content["result"][0]["rating"]), | ||
| config["contri"] = str(html_content["result"][0]["contribution"]), | ||
| config["rank"] = str(html_content["result"][0]["rank"]), | ||
| config["maxrating"] = str(html_content["result"][0]["maxRating"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can create a single object with name profile, and put above as key value pairs, and then write that object.
| * ``` | ||
| ecp config set_user <username> | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't it be 'set-user' instead of 'set_user'?
|
Have made the recommended changes |
|
Kindly add proxy support to it. |
|
src/Cli/config.py have called request from self.get_proxy() ,and it works properly. |
src/Config/Config.py
Outdated
| config_file.write(json.dumps(config)) | ||
|
|
||
| def set_user(self, user): | ||
| response = requests.get(url='http://codeforces.com/api/user.info?handles='+user,proxies=self.get_proxy()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can store the url in a variable first and then use it to improve readability
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not done yet
|
done |
| try: | ||
| config = Config() | ||
| config.get_user() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not printing anything
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
corrected it
src/Config/Config.py
Outdated
| address='http://codeforces.com/api/user.info?handles='+user | ||
| response = requests.get(url=address,proxies=self.get_proxy()) | ||
| if(response.status_code!=200): | ||
| raise UsernameError('User not Found') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No implementation of UsernameError class. Also I think "UsernameNotFound" would be a better name
src/Cli/config.py
Outdated
| try: | ||
| config = Config() | ||
| config.get_user() | ||
| print(config.get_user()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use click.echo with styling.
src/Config/Config.py
Outdated
| response = requests.get(url=address,proxies=self.get_proxy()) | ||
| if(response.status_code!=200): | ||
| raise UsernameError('User not Found') | ||
| raise UsernameError('UsernameNotFound') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was saying that there is no such class as "UsernameError". You have to create it on your own. Also I think the name of class should be "UsernameNotFound" with error string as "User not found".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
src/Config/Config.py
Outdated
| class UsernameNotFound(Exception): | ||
| def __init__(self, msg): | ||
| super().__init__(self, msg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the previous review as well I said shouldn't it have a default msg = "User not found"?
Also, kindly make a separate directory for exceptions related to profile and write class over there. Look into "Runner" directory, you can find an example there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sir please check the changes now.
|
When using "get-user" we can raise an exception when user is not set earlier. |
|
When using "get-user" we can now raise an exception when user is not set earlier. |
src/Config/Config.py
Outdated
| with open(config_file_path, 'r') as config_file: | ||
| config = json.load(config_file) | ||
| if(len(config["user"]["firstname"])==0): | ||
| raise UserNotSet |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you not testing your commands after writing code?? Why raising a class??
Also, put proper spacing between functions and wherever you find it necessary.
and
fixed error
HTTPSConnectionPool(host='codeforces.com', port=443): Max retries exceeded with url: /api/user.info?handles=touris
(Caused by ProxyError('Your proxy appears to only use HTTP and not HTTPS, try changing your proxy URL to be HTTP.
See: https://urllib3.readthedocs.io/en/1.26.x/advanced-usage.html#https-proxy-error-http-proxy',
SSLError(SSLError(1, '[SSL: WRONG_VERSION_NUMBER] wrong version number (_ssl.c:1129)'))))
|
Sir please check the changes made now. |
|
Receiving a proxy authentication error. Are you facing the same problem? |
|
It was due to an update in the requests library, I have made the necessary changes. |
src/Cli/config.py
Outdated
| config = Config() | ||
| print(config.get_user()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably overwritten by some new commit. Use click.echo here. Also, consider formatting this instead of simply printing the object.
src/Config/exceptions/UserNotSet.py
Outdated
| @@ -0,0 +1,3 @@ | |||
| class UserNotSet(Exception): | |||
| def __init__(self) -> None: | |||
| super().__init__("User not set yet.") No newline at end of file | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need of full stop
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need of full stop
src/Cli/config.py
Outdated
| click.echo(e) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can use click styling to change color to red
src/Config/config.json
Outdated
| @@ -1 +1 @@ | |||
| {"test_in_progress": "False", "language": "python", "template": "D:\\Alok country\\Apun coder banega\\Projects\\CP-CLI\\test\\code.cpp", "proxy": ""} No newline at end of file | |||
| {"user": {"firstname": "", "lastname": "", "rating": [""], "contri": [""], "rank": [""], "maxrating": ""}, "test_in_progress": "False", "language": "python", "template": "D:\\Alok country\\Apun coder banega\\Projects\\CP-CLI\\test\\code.cpp", "proxy": ""} No newline at end of file | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why storing rating, contri, rank and maxrating in an array?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it will get easier if we call get-user method anywhere else in the code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't get it. How come it will become easier?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just tried to encapsulate user in one key value pair.
I thought it be easier to call property of user from this key value pair, instead of calling individual properties of user
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a good approach. Probably you are not getting my question. What I an asking is why is your rating property in user object an array? As you can see it's written that "rating": [""]. Why this instead of "rating" : "".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had the same doubt actually, when I call it, it comes in this form only.
ashutoshsuthar2020
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Applied recommended changes
| try: | ||
| config = Config() | ||
| config.get_user() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
corrected it
src/Config/Config.py
Outdated
| response = requests.get(url=address,proxies=self.get_proxy()) | ||
| if(response.status_code!=200): | ||
| raise UsernameError('User not Found') | ||
| raise UsernameError('UsernameNotFound') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
src/Config/Config.py
Outdated
| class UsernameNotFound(Exception): | ||
| def __init__(self, msg): | ||
| super().__init__(self, msg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sir please check the changes now.
src/Config/config.json
Outdated
| @@ -1 +1 @@ | |||
| {"test_in_progress": "False", "language": "python", "template": "D:\\Alok country\\Apun coder banega\\Projects\\CP-CLI\\test\\code.cpp", "proxy": ""} No newline at end of file | |||
| {"user": {"firstname": "", "lastname": "", "rating": [""], "contri": [""], "rank": [""], "maxrating": ""}, "test_in_progress": "False", "language": "python", "template": "D:\\Alok country\\Apun coder banega\\Projects\\CP-CLI\\test\\code.cpp", "proxy": ""} No newline at end of file | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it will get easier if we call get-user method anywhere else in the code
src/Config/Config.py
Outdated
| config["user"]["firstname"] = html_content["result"][0]["firstName"] | ||
| config["user"]["lastname"] = html_content["result"][0]["lastName"] | ||
| config["user"]["rating"] = str(html_content["result"][0]["rating"]), | ||
| config["user"]["contri"] = str(html_content["result"][0]["contribution"]), | ||
| config["user"]["rank"] = str(html_content["result"][0]["rank"]), | ||
| config["user"]["maxrating"] = str(html_content["result"][0]["maxRating"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's an error over here which is causing creation of a list. Try to figure it out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The commas 😢
removed em
src/Cli/config.py
Outdated
| def get_user(): | ||
| try: | ||
| config = Config() | ||
| click.echo(config.get_user()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As asked in the last review as well, why are you directly printing the object. Can you consider formatting it?
src/Config/Config.py
Outdated
| return config["user"] | ||
| for info in config["user"]: | ||
| print(info + " : " + str(config["user"][info])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Earlier approach was correct. Receive user object from Config.py and print it in config.py
Added a profile command which allows you to make profile card of the given user
For example
ecp profile tourist
---------------------
User Name : GennadyKorotkevich
Rating : 3834
Contribution : 134
Rank : legendary grandmaster
Max Rating : 3979
---------------------