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

Fix IntegrityError for 5.3.X branch #1058

Merged
merged 4 commits into from
Feb 2, 2017
Merged

Conversation

Natim
Copy link
Member

@Natim Natim commented Feb 2, 2017

Fixes #1055

@Natim Natim requested a review from leplatrem February 2, 2017 13:32
with self.client.connect() as conn:
conn.execute(query, dict(key=self.prefix + key,
value=value, ttl=ttl))
server_version = conn.connection().dialect.server_version_info
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: here https://github.com/zzzeek/sqlalchemy/blob/bc4a96836d5bfb911da26f7e77119f3a7b356f2e/lib/sqlalchemy/testing/exclusions.py#L384 they use getattr so maybe we should use it too and compare version >= (9, 5)

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think we need to use getattr there because we are always using the pgdialect that will always define it.

@leplatrem
Copy link
Contributor

Quick note on maintainance: I'd rather continue to submit patches to the master branch. Then, when we want to release 5.3.X, we backport the pull-request (usually with a simple git cherry-pick -m 1 <merge-commit>). Duplicating the PRs like this and #1056 can end-up being confusing IMO

@Natim
Copy link
Member Author

Natim commented Feb 2, 2017

I agree with you generally but for this specific case the patch are different between the master 6.X and the 5.X branch

@leplatrem
Copy link
Contributor

patch are different between the master 6.X and the 5.X branch

Gotcha, but maybe we can handle the situation in the content of the commit that backports the change? (not sure what's best, let's see next time...).

@Natim Natim merged commit b51fedc into 5.3.X Feb 2, 2017
@Natim Natim deleted the 1055-integrity-error-5.3.X branch February 2, 2017 17:03
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