Skip to content

typing - packet.py #73

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

Conversation

heehehe
Copy link
Member

@heehehe heehehe commented Aug 29, 2023

3조 typing 결과는 각 PR을 feature/typing-packet-gtid-binlogstream 브랜치로 합쳐서 진행하고 있습니다🙂

To Do

  • pytest (actions 오류 확인)
  • 누락된 typing 확인 - instance 변수 typing / __init__
  • docstring 확인

@heehehe heehehe self-assigned this Aug 29, 2023
@heehehe heehehe added the 3조 3조 만세 label Aug 29, 2023
@dongwook-chan
Copy link

dongwook-chan commented Sep 1, 2023

여러개의 타입 중 하나가 올 수 있는 경우 Tuple 사용하라고 설명 드렸던 조가 3조였던 것 같은데... 잘못된 정보 알려드려서 죄송합니다.
다들 스마트하셔서 실제 코드 적용하실 때는 Union으로 잘 적용해주셨군요! 함께 말씀드렸던 |는 찾아보니 python3.10부터만 지원되기 때문에 저희 테스트 환경인 3.7에서도 돌 수 있도록 Union으로 통일하는 것이 좋을 것 같습니다!

Copy link

@dongwook-chan dongwook-chan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

제가 잘못 설명드렸음에도 불구하고 찰떡같이 알아들으시고 스마트하게 typing 잘 해주셨습니다! 3조 정말 매번 열심히 해주셔서 정말 감사합니다! 🙇🙇

@@ -4,6 +4,9 @@

from pymysqlreplication import constants, event, row_event

from typing import List, Tuple, Dict, Optional, Union
from pymysql.connections import MysqlPacket

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any라고 적고 끝내실수도 있는데 끈질기게 해법을 고민해주셨다는 게 대단한 것 같아요!
끈기는 개발자의 핵심 역량이죠 👍👍👍
다음번에도 막히는 부분 있다면 머리를 맞대어 해결해봅시다!!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

정승님께서 발견해주신 덕분에 잘 바꿀 수 있었습니다ㅎㅎ 넘 감사합니다!!😆

@@ -498,3 +494,18 @@ def read_string(self):
string += char

return string


def read_offset_or_inline(packet: Union[MysqlPacket, BinLogPacketWrapper], large: bool) \
Copy link
Member

@starcat37 starcat37 Sep 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

일단 packet.py가 쉽지 않아 보이던데 엄청 꼼꼼하게 해주셔서 감탄하면서 봤네요 😲😲 고생하셨어요!! 🥰🥰

혹시 이 함수가 밑으로 옮겨진 이유가 따로 있을까요? 오류는 아니고 궁금해서 여쭤봅니다...!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

감사합니다🥺
packet type으로 BinLogPacketWrapper를 사용해야 하는데 위에 있으면 이게 정의되어 있지 않다고 나오더라구요..!
그래서 아래로 옮겨뒀습니다 :) (upstream PR 올릴 때 코멘트로 설명 달아야겠네요!)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아하 그렇군요!! 알려주셔서 감사해요🥰🥰🥰

@mjs1995
Copy link
Member

mjs1995 commented Sep 3, 2023

고생 많으셨어요! Typing을 엄청 꼼꼼하게 잘 해주신거 같아서 한 수 배워가네요 👍

@mikaniz
Copy link

mikaniz commented Sep 3, 2023

수고 많으셨습니다!! 정말 꼼꼼한 Typing 인 것 같아요! 많이 배웠습니다~~!!!

@heehehe heehehe changed the base branch from feature/typing-packet-gtid-binlogstream to feature/typing-gtid-exception-binlogstream-packet September 5, 2023 01:39
@heehehe heehehe merged commit cc6ac07 into feature/typing-gtid-exception-binlogstream-packet Sep 5, 2023
Comment on lines +260 to 266
def read_length_coded_pascal_string(self, size: int) -> Union[int, bytes]:
"""
Read a string with length coded using pascal style.
The string start by the size of the string
"""
length = self.read_uint_by_size(size)
return self.read(length)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

어떤경우에 int가 나오고 어떤경우 byte가나오나요?

Copy link
Member Author

@heehehe heehehe Sep 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

원래는 아래 부분에서 if len(self.__data_buffer) > 0이면 bytes로 되고 아니면 int로 된다고 생각했는데요..
https://github.com/23-OSSCA-python-mysql-replication/python-mysql-replication/blob/4ccf81ad8024c43bfd9b7b0e8ffbca51e3657e86/pymysqlreplication/packet.py#L154-L164

pymysql 코드 살펴보니까 모두 bytes로 반환될 것 같네요..!
https://github.com/PyMySQL/PyMySQL/blob/8157da51e844f619eb693c5f5dd2758dca1d1c98/pymysql/protocol.py#L62-L76

자세히 살펴봐주셔서 감사합니다 수정해볼게요!! - 4c1286c 반영 완료

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

Successfully merging this pull request may close these issues.

6 participants