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

User specific schema configurations #201

Open
wants to merge 253 commits into
base: feature/schema-tools
Choose a base branch
from

Conversation

raheelriax
Copy link

Added Customized schema configurations for both
Transaction and version tables.
You can now pass schema name as an option to make_versioned method or define it explicitly in versioned attribute of parent model.
If no configuration is passed it will use default configurations

Fernando Cezar and others added 16 commits May 16, 2018 16:47
Hotfix/track cloned connections (kvesteri#2)

Hotfix/track cloned connections (kvesteri#3)

change iterations

use only ConnectionFairy as indexes

create new dict entry for clones

create entry using old connection

create entry using session

revert to event listening strategy

remove cloned connections on rollback

treat None case when cleaning connections

Hotfix/track cloned connections (kvesteri#2)

Hotfix/track cloned connections (kvesteri#3)

change iterations

use only ConnectionFairy as indexes

create new dict entry for clones

create entry using old connection

create entry using session

revert to event listening strategy

remove cloned connections on rollback

treat None case when cleaning connections
Raising a new KeyError means you lose the original stacktrace, which
is going to be more useful since it'll contain more information about
the relevant context for this error.
…ption

Reraise original exception instead of a new one
Fix ResourceClosedErrors from connections leaking when using an external transaction
@chris-mouthwatch
Copy link

@raheelriax is there any estimate on when this PR could be merged?

@chris-mouthwatch
Copy link

pinging @kvesteri on this.

@marksteward
Copy link
Collaborator

This looks like it needs some serious rebasing.

@chris-mouthwatch
Copy link

@marksteward @kvesteri do you have any suggestions on how to generate transaction_id in a custom way rather than using seq?
We are using schema in postgresql and I think:

  • for long-term, this PR will resolve the issue.
  • for short-term, maybe using UUID or something like that could be used instead of transaction_id_seq.

What do you think?

@marksteward
Copy link
Collaborator

What aspect of this PR will fix that? I'm happy to look at a smaller PR.

I don't think UUID makes sense as the primary key for Transaction, you'd still need some way of sequencing them that's stable and unambiguously ordered (i.e. not date).

Why can't you use a sequence currently? I think we might be able to remove the explicit Sequence in recent versions of SQLAlchemy if that helps.

@chris-mouthwatch
Copy link

@marksteward
So the problem is that we have applied versioning to the tables in communication schema in postgres.
The problem is that it seems like it's trying to use public.transaction_id_seq instead of communication.transaction_id_seq and that's why we are seeing this error.
image
image

@chris-mouthwatch
Copy link

So if we can append schema name i.e. communication.transaction_id_seq then it would resolve the issue.
What do you think, @marksteward ?

@chris-mouthwatch
Copy link

Actually, adding this in the back resolved the issue.
image

@marksteward
Copy link
Collaborator

I think we might just need to reference Base.metadata in the Sequence declaration. I'll give that a go this evening.

@chris-mouthwatch
Copy link

Great, thanks.

@jun-828
Copy link

jun-828 commented Aug 10, 2022

Thanks, guys. @marksteward I think, one of these two will work:

  • Reference the schema information in Base.metadata as you mentioned.
  • If first option will take too much effort, then we can just remove nextVal(transaction_seq_id) when we run INSERT INTO transactions as it is already autoincrement int type.

In anyway, I can't wait to see this fix coming. Thanks for all your supports btw.

@chris-mouthwatch
Copy link

@marksteward any chance that you could take a look on this?

@chris-mouthwatch
Copy link

@marksteward did you have a chance to look into this one?
You can just give me a hint and I can try

@marksteward
Copy link
Collaborator

I've not forgotten about this but ran out of time today.

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.