Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the dining crawling system to support multiple cookies (JSESSIONID and KSMSID) instead of a single cookie, and modifies Redis integration to store and retrieve both cookies.
- Added LOGIN_COOKIES configuration defining the two required cookies with their respective domains
- Updated cookie retrieval and storage logic to handle multiple cookies as a dictionary
- Modified the send_request function to construct cookies from a dictionary structure
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| crawling/koreatech_portal/login_v2.py | Added debug print statement for the new KSMSID cookie |
| crawling/koreatech_portal/dining.py | Introduced multi-cookie support with LOGIN_COOKIES configuration, updated send_request to accept cookie dictionary, and modified get_cookie function to handle multiple cookies from Redis and portal login |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def send_request(login_cookies, eat_date, eat_type, restaurant, campus): | ||
| headers = {"Content-Type": "text/xml; charset=utf-8"} | ||
| cookies = {"JSESSIONID": f"{login_cookie};Domain=kut90.koreatech.ac.kr;"} | ||
| cookies = {cookie['name']: f"{login_cookies[cookie['name']]};Domain={cookie['domain']}" for cookie in LOGIN_COOKIES} |
There was a problem hiding this comment.
The parameter name changed from 'login_cookie' (singular) to 'login_cookies' (plural), but the dictionary comprehension uses 'login_cookies[cookie['name']]' which assumes login_cookies is a dictionary. However, based on the function signature, this creates confusion about the expected data structure. Consider renaming the parameter to 'cookie_dict' or 'cookies_dict' to clearly indicate it's a dictionary mapping cookie names to values.
| for name in login_cookie: | ||
| if not login_cookie[name]: continue | ||
| login_cookie[name] = login_cookie[name].decode("utf-8") | ||
| try: | ||
| send_request(login_cookie, datetime.today().strftime("%Y-%m-%d"), "lunch", "한식", "Campus1") | ||
| return login_cookie | ||
| except ConnectionError: | ||
| pass | ||
|
|
There was a problem hiding this comment.
The validation logic only checks if cookies exist but doesn't verify they are complete before attempting to use them. If one cookie is missing from Redis (returns None), it will be included in the dictionary with a None value, and line 169's send_request call will fail. The validation should ensure all required cookies are present before attempting to use them.
| for name in login_cookie: | |
| if not login_cookie[name]: continue | |
| login_cookie[name] = login_cookie[name].decode("utf-8") | |
| try: | |
| send_request(login_cookie, datetime.today().strftime("%Y-%m-%d"), "lunch", "한식", "Campus1") | |
| return login_cookie | |
| except ConnectionError: | |
| pass | |
| # 모든 필수 쿠키가 존재하는지 확인하면서 디코딩 | |
| all_cookies_present = True | |
| for name, value in login_cookie.items(): | |
| if value is None: | |
| all_cookies_present = False | |
| break | |
| login_cookie[name] = value.decode("utf-8") | |
| # 캐시된 쿠키가 완전한 경우에만 검증 요청 수행 | |
| if all_cookies_present: | |
| try: | |
| send_request(login_cookie, datetime.today().strftime("%Y-%m-%d"), "lunch", "한식", "Campus1") | |
| return login_cookie | |
| except ConnectionError: | |
| pass |
| def send_request(login_cookies, eat_date, eat_type, restaurant, campus): | ||
| headers = {"Content-Type": "text/xml; charset=utf-8"} | ||
| cookies = {"JSESSIONID": f"{login_cookie};Domain=kut90.koreatech.ac.kr;"} | ||
| cookies = {cookie['name']: f"{login_cookies[cookie['name']]};Domain={cookie['domain']}" for cookie in LOGIN_COOKIES} |
There was a problem hiding this comment.
The dictionary comprehension will raise a KeyError if any cookie value from portal_login() is None or missing. This occurs when accessing login_cookies[cookie['name']] on line 90 before the cookie values are validated. The code should handle cases where cookies might be None or missing from the login_cookies dictionary.
| cookies = {cookie['name']: f"{login_cookies[cookie['name']]};Domain={cookie['domain']}" for cookie in LOGIN_COOKIES} | |
| cookies = {} | |
| for cookie in LOGIN_COOKIES: | |
| name = cookie.get("name") | |
| domain = cookie.get("domain") | |
| if not name or not domain: | |
| continue | |
| value = login_cookies.get(name) | |
| if value: | |
| cookies[name] = f"{value};Domain={domain}" |
| login_cookie = portal_login().get('JSESSIONID', domain='kut90.koreatech.ac.kr') | ||
| redis_client.set('JSESSIONID', login_cookie) | ||
| cookies = portal_login() | ||
| login_cookie = {cookie['name']: cookies.get(cookie['name'], domain=cookie['domain']) for cookie in LOGIN_COOKIES} |
There was a problem hiding this comment.
Similar to the issue on line 90, this line will raise a KeyError if portal_login() doesn't return all expected cookies. If cookies.get() returns None for any cookie, the dictionary will contain None values which will then cause redis_client.set() on line 178 to fail. The code should validate that all required cookies were successfully obtained before attempting to store them.
| login_cookie = {cookie['name']: cookies.get(cookie['name'], domain=cookie['domain']) for cookie in LOGIN_COOKIES} | |
| login_cookie = {cookie['name']: cookies.get(cookie['name'], domain=cookie['domain']) for cookie in LOGIN_COOKIES} | |
| # 필수 쿠키가 모두 존재하는지 확인 | |
| missing_cookies = [cookie['name'] for cookie in LOGIN_COOKIES if login_cookie.get(cookie['name']) is None] | |
| if missing_cookies: | |
| raise ConnectionError(f"필수 로그인 쿠키를 모두 가져오지 못했습니다: {', '.join(missing_cookies)}") |
| for name in login_cookie: | ||
| if not login_cookie[name]: continue | ||
| login_cookie[name] = login_cookie[name].decode("utf-8") | ||
| try: | ||
| send_request(login_cookie, datetime.today().strftime("%Y-%m-%d"), "lunch", "한식", "Campus1") | ||
| return login_cookie | ||
| except ConnectionError: | ||
| pass | ||
|
|
There was a problem hiding this comment.
The continue statement on line 166 should be followed by proper handling. When a cookie is None after Redis retrieval, the code continues but leaves that cookie as None in the dictionary. This means send_request on line 169 will receive incomplete cookies and fail. Either all cookies should be present for validation to succeed, or the code should fall through to re-login immediately when any cookie is missing.
| for name in login_cookie: | |
| if not login_cookie[name]: continue | |
| login_cookie[name] = login_cookie[name].decode("utf-8") | |
| try: | |
| send_request(login_cookie, datetime.today().strftime("%Y-%m-%d"), "lunch", "한식", "Campus1") | |
| return login_cookie | |
| except ConnectionError: | |
| pass | |
| # Redis에서 읽어온 쿠키를 디코딩하되, 존재하지 않는 쿠키는 그대로 둔다. | |
| for name, value in login_cookie.items(): | |
| if not value: | |
| continue | |
| login_cookie[name] = value.decode("utf-8") | |
| # 모든 쿠키가 존재할 때만 검증 요청을 보낸다. | |
| .all_cookies_present = all(login_cookie[name] for name in login_cookie) | |
| if all_cookies_present: | |
| try: | |
| send_request(login_cookie, datetime.today().strftime("%Y-%m-%d"), "lunch", "한식", "Campus1") | |
| return login_cookie | |
| except ConnectionError: | |
| # 검증 실패 시 재로그인 절차로 진행 | |
| pass |
No description provided.