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

chore(docs): Fix quick start example #289

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

franz101
Copy link

@franz101 franz101 commented Jan 24, 2024

The current example leads to the following error with sqlalchemy > 2:
TypeError: MetaData.__init__() got an unexpected keyword argument 'bind'

Maybe fixes this:
#261

@franz101
Copy link
Author

@xzkostyan do you mind adding me as a maintainer or we find a way to get small improvements through?

@xzkostyan
Copy link
Owner

Let's start with a little PR review.

Why we use pure functions instead clickhouse-dialect wrappers here?

It seems that transferring bind=engine from MetaData to .create() call is only related change in this PR. What other changes are about?

It will not fix #261. For some reasons sqlalchemy_utils are used there.

@franz101
Copy link
Author

I see what you mean, thanks for the feedback:



engine = create_engine(uri)

# Base = get_declarative_base(metadata=metadata)

# Create a session.
Session = sessionmaker(bind=engine)
session = Session()
metadata = MetaData()
metadata.reflect(bind=engine)

# Define the base class using the declarative base.
Base = declarative_base(metadata=metadata)

this should reflect the metadata part?

Can you explain the difference between clickhouse dialect wrappers?

@franz101
Copy link
Author

In general I was assuming clickhouse 2.0 was breaking the docs

@franz101
Copy link
Author

This seems to work for now:

metadata = MetaData()
metadata.reflect(bind=engine)

Base = get_declarative_base(metadata=metadata)

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.

None yet

2 participants