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

#166 use the new version of AtCoder #358

Merged
merged 3 commits into from
Mar 8, 2019
Merged

#166 use the new version of AtCoder #358

merged 3 commits into from
Mar 8, 2019

Conversation

kmyk
Copy link
Member

@kmyk kmyk commented Mar 6, 2019

close #166

旧AtCoderに依存していた部分を新AtCoderで置き換えます。
@fukatani コードの他に、AtCoderから送られてくるエラーメッセージの処理が正しく動いてそうか簡単にでよいので確認してほしいです。

@kmyk kmyk added this to the v6.0.0 milestone Mar 6, 2019
@kmyk kmyk requested a review from fukatani March 6, 2019 14:08
@fukatani
Copy link
Contributor

fukatani commented Mar 8, 2019

#363 は手元でワークアラウンドで対処して動作確認しました。

ログイン失敗

Username: ryoryoryo111
Password: 
[x] POST: https://atcoder.jp/login
[x] 200 OK
[!] AtCoder says: × Username or Password is incorrect.
[-] Username or Password is incorrect.
[x] save cookie to: /home/ryo/.local/share/online-judge-tools/cookie.jar

ログイン成功

Username: ryoryoryo111
Password: 
[x] POST: https://atcoder.jp/login
[x] redirected: https://atcoder.jp/
[x] 200 OK
[!] AtCoder says: × Welcome, ryoryoryo111.
[+] Welcome,

@fukatani
Copy link
Contributor

fukatani commented Mar 8, 2019

@kmyk
他になにか入れるべき入力はあるでしょうか?

@fukatani
Copy link
Contributor

fukatani commented Mar 8, 2019

存在しないURLにダウンロードとかやると例外で落ちるので、ダウンロードのalertが起きる条件がわかってません。

@kmyk
Copy link
Member Author

kmyk commented Mar 8, 2019

他になにか入れるべき入力はあるでしょうか?

他のエラーとしては提出頻度制限がありますが、ログイン部分でエラー取得が動いているなら他の確認は不要のはずです。

存在しないURLにダウンロードとかやると例外で落ちるので、ダウンロードのalertが起きる条件がわかってません。

以下の部分であれば、これはデグレです。ミスなので修正とテストの追加をします。
以前はAtCoderがすべて200を返してしまっていたのでこのような検査が必要でしたが、新AtCoderになって404が帰るようになったようです。

https://github.com/kmyk/online-judge-tools/blob/ff8865c55cbb1ee762ed19713276e76b2671cbf9/onlinejudge/service/atcoder.py#L309-L312

@fukatani
Copy link
Contributor

fukatani commented Mar 8, 2019

ログインしてないときにsubmitすると例外で落ちるのは仕様でしたっけ?

@kmyk
Copy link
Member Author

kmyk commented Mar 8, 2019

method から NotLoggedInError として例外が飛ぶ部分は仕様で、コマンドを実行したときにこれを catch できてないのはバグです。これも修正します。

@fukatani
Copy link
Contributor

fukatani commented Mar 8, 2019

ユーザーのありうる入力はキャッチしたいということですね。

存在しないファイルをsubmitするとFileNotFoundErrorで落ちますが、これもバグでしょうか?

@kmyk
Copy link
Member Author

kmyk commented Mar 8, 2019

FileNotFoundError もたしかに catch したいですね。通常ありえる範囲の操作をしていてユーザから例外の文字列が見えるのは適当でないので、これもバグと言っていいと思います。

@kmyk
Copy link
Member Author

kmyk commented Mar 8, 2019

まずいところが4箇所見付かりました。最終的にすべて修正されるべきですが、このプルリクの責任範囲は (1) だけのはずなので、修正はとりあえずこれだけをします。特に (2) (3) は「異常系の表示が汚い」というだけで優先度が低いはずなので、Issueに書いて後回しにしたいです。

  1. 存在しないURLにダウンロードとかやると例外
  2. ログインしてないときにsubmitすると例外
  3. 存在しないファイルをsubmitすると例外
  4. Runtime error on Python 3.5.2 #363

@fukatani
Copy link
Contributor

fukatani commented Mar 8, 2019

はい、NotLoginErrorもFileNotFoundErrorも前からですし、このPRのスコープ外でいいと思います。

@kmyk
Copy link
Member Author

kmyk commented Mar 8, 2019

alertのまわりの実装が変だったのを修正し、例外が飛ぶことをdocstringに明記しました。
少なくとも download_sample_cases のmethodはこれで正しいものとなったはずです。

できれば ProblemNotFoundError のような例外クラスを切るべきですが、とりあえず最も抽象的な Exception をdocstring書いてあります。
飛んだ例外がユーザに見えてしまう問題は #365 で解決するつもりです。

@kmyk
Copy link
Member Author

kmyk commented Mar 8, 2019

@fukatani こちらで修正が必要なものはこれだけのはずなので、レビューの続きや、よさそうならマージお願いします

@kmyk
Copy link
Member Author

kmyk commented Mar 8, 2019

(ところで4個も見付かるのすごい。助かります)

@fukatani
Copy link
Contributor

fukatani commented Mar 8, 2019

珍しく(?)真面目に動作確認したら色々見つかってびっくりしました。

というわけでLGTMです。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

download, submit, login: use new atcoder.jp pages
2 participants