Skip to content

Conversation

lxyu
Copy link
Contributor

@lxyu lxyu commented Jul 7, 2014

a 2nd PR continues on the previous one.

This PR will expose primary keys info in row-based events, so we may only listen/publish the primary key values.

@bjoernhaeuser
Copy link
Collaborator

👍 I am fine with that, will merge it when we finished #65

@julien-duponchelle
Copy link
Owner

👍 I'm fine too

@bjoernhaeuser
Copy link
Collaborator

Hm, PRIMARY KEYs could be more than one column. Is this addressed? Could not spot it :)

lxyu added 2 commits July 8, 2014 15:19
in favor of sqlalchemy's primary_key definition style
@@ -3,12 +3,21 @@

class Table(object):
def __init__(self, column_schemas, table_id, schema, table, columns):
primary_key = [c.data["name"] for c in columns if c.data["is_primary"]]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There may be multiple primary keys or none primary key here, so the final primary_key can either be str or tuple.

I convert it to tuple here so it's not mutable.

@lxyu
Copy link
Contributor Author

lxyu commented Jul 9, 2014

It should be fixed now. Please review it again, thanks.

@bjoernhaeuser
Copy link
Collaborator

👍

@bjoernhaeuser
Copy link
Collaborator

@noplay shall we merge it?

@bjoernhaeuser
Copy link
Collaborator

@lxyu You can make a new pull request in which you add yourself to the contributors list

@lxyu
Copy link
Contributor Author

lxyu commented Jul 9, 2014

@bjoernhaeuser I'm already added. The "Lx Yu" is me ;)

@bjoernhaeuser
Copy link
Collaborator

okay :) thats good to know :)

@julien-duponchelle
Copy link
Owner

Yeah we can merge it :)

julien-duponchelle added a commit that referenced this pull request Jul 9, 2014
expose primay_keys info for row based events
@julien-duponchelle julien-duponchelle merged commit 9cec971 into julien-duponchelle:master Jul 9, 2014
@lxyu lxyu deleted the primay_col_info branch July 10, 2014 03:42
@julien-duponchelle julien-duponchelle added this to the 0.4 milestone Jul 11, 2014
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.

3 participants