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

Feature/mariadb gtid list event #16

Conversation

chungeun-choi
Copy link

mariadb gtid list event 구현 - #4

@chungeun-choi chungeun-choi self-assigned this Jul 23, 2023
@chungeun-choi chungeun-choi added 0조 최강 0조 enhancement New feature or request labels Jul 23, 2023
@chungeun-choi chungeun-choi changed the base branch from main to feature/mariadb-gtid-list-event July 25, 2023 14:09
@dongwook-chan dongwook-chan merged commit 8357c53 into python-mysql-replication-kr:feature/mariadb-gtid-list-event Jul 25, 2023
@dongwook-chan
Copy link

dongwook-chan commented Jul 25, 2023

아래 테스트 fail 되었네요.
https://github.com/23-OSSCA-python-mysql-replication/python-mysql-replication/actions/runs/5658889179/job/15331201088#step:6:31

아래 테스트에서 클래스 개수 수정해주셔야할 것 같습니다.
https://github.com/23-OSSCA-python-mysql-replication/python-mysql-replication/blob/f8b862ab4dd53b14e70c852fc6df11b149ff2a1f/pymysqlreplication/tests/test_basic.py#L28-L31

다음 순서대로 진행해주시면 될 것 같습니다~

feature/mariadb-gtid-list-event 브랜치에서 먼저 수정 -> PR 생성 -> merge
feature/mariadb-annotate-rows-event 브랜치에서 수정 -> PR 생성 -> merge

@chungeun-choi
Copy link
Author

chungeun-choi commented Jul 25, 2023

앗...event 갯수 늘어났다는걸 놓쳤었네요 수정해서 제가 'feature/annotate-rows-event' 브랜치에 merge까지 눌러놓겠습니다!

Choose a reason for hiding this comment

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

크... Pythonic하게 잘 짜셨네요 👍🏻





Choose a reason for hiding this comment

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

사소한거긴 하지만 파일 마지막에는 빈 라인 하나만 넣어주세요~
Too many blank lines (3) (E303)

def __init__(self, from_packet, event_size, table_map, ctl_connection, **kwargs):
super(MariadbGtidObejct, self).__init__(from_packet, event_size, table_map, ctl_connection, **kwargs)
self.domain_id = self.packet.read_uint32()
self.server_id = self.packet.server_id

Choose a reason for hiding this comment

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

여기에서도 unsined int 4 bytes 읽어주셔야(read_uint32()) 합니다 ㅜㅜ

self.assertIsInstance(event,FormatDescriptionEvent)

#MariadbAnnotateRowsEvent
event = self.stream.fetchone()

Choose a reason for hiding this comment

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

최소한 1개의 필드값에 대해서라도 assert 해줘야할 것 같아요
이벤트 발생시켜서 트랜잭션 번호 올린다음에 구현해주신 MariadbGtidObejct가 잘 들어왔는지
assert하면 될 것 같습니다.

--default-authentication-plugin=mysql_native_password
--log-bin=master-bin
--binlog-format=row
--log-slave-updates=on

Choose a reason for hiding this comment

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

사소한 것이지만 깃으로 형상 관리중인 파일은 맨 마지막에 빈칸 하나 넣어주세요~

self.assertEqual(event.event_type,15)
self.assertIsInstance(event,FormatDescriptionEvent)

#MariadbAnnotateRowsEvent

Choose a reason for hiding this comment

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

주석의 이벤트명 틀린 것 같습니다!

@@ -67,7 +67,7 @@ def isMySQL80AndMore(self):

def isMariaDB(self):
if self.__is_mariaDB is None:
self.__is_mariaDB = "MariaDB" in self.execute("SELECT VERSION()").fetchone()
self.__is_mariaDB = "MariaDB" in self.execute("SELECT VERSION()").fetchone()[0]

Choose a reason for hiding this comment

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

👍🏻👍🏼👍🏽👍🏾👍🏿

@notion-workspace
Copy link

Untitled

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0조 최강 0조 enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants