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

bootstrap_servers argument usage #98

Merged

Conversation

brunos252
Copy link
Contributor

@brunos252 brunos252 commented Mar 7, 2022

Update the bootstrap_servers argument to accept unquoted values and lists of unquoted values.

@brunos252 brunos252 added the status: ready PR is ready for review label Mar 7, 2022
@brunos252 brunos252 requested a review from g-despot March 7, 2022 14:57
@brunos252 brunos252 self-assigned this Mar 7, 2022
@brunos252 brunos252 linked an issue Mar 7, 2022 that may be closed by this pull request
gqlalchemy/models.py Outdated Show resolved Hide resolved
servers_field = f"'{self.bootstrap_servers}'"
else:
servers_field = str(self.bootstrap_servers)[1:-1]
query += f" BOOTSTRAP_SERVERS {servers_field}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider changing this if-else into one-liner

Copy link
Contributor Author

Choose a reason for hiding this comment

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

can you help me understand how? Something like:
servers_field = f"'{self.bootstrap_servers}'" if isinstance(self.bootstrap_servers, str) else str(self.bootstrap_servers)[1:-1]
Seems too long

Copy link
Contributor

Choose a reason for hiding this comment

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

Python is known for such long statements. But i stated "consider", not "rewrite"

Copy link
Contributor

@BorisTasevski BorisTasevski left a comment

Choose a reason for hiding this comment

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

Minor updates

@brunos252 brunos252 changed the base branch from main to develop March 8, 2022 14:44
@brunos252
Copy link
Contributor Author

Build and Test fails because of pymgclient version 1.1.0 in pyproject.toml and poetry.lock, this is not an issue here as it is like this in develop branch

@g-despot g-despot merged commit d23c67a into develop Mar 16, 2022
@g-despot g-despot deleted the 93-allow-bootstrap_servers-argument-usage-without-quotes branch March 16, 2022 08:33
@g-despot g-despot restored the 93-allow-bootstrap_servers-argument-usage-without-quotes branch March 16, 2022 08:34
@brunos252 brunos252 deleted the 93-allow-bootstrap_servers-argument-usage-without-quotes branch March 16, 2022 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready PR is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow bootstrap_servers argument usage without quotes
4 participants