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

IdentifierGenerator(Factory) related work #4429

Closed
wants to merge 5 commits into from

Conversation

sebersole
Copy link
Member

HHH-14497 - Drop legacy id-generator settings;
HHH-14718 - Drop deprecated generator implementations;
HHH-14959 - Drop IdentifierGeneratorFactory as a Service

HHH-14718 - Drop deprecated generator implementations;
HHH-14959 - Drop IdentifierGeneratorFactory as a Service
HHH-14718 - Drop deprecated generator implementations;
HHH-14959 - Drop IdentifierGeneratorFactory as a Service;
HHH-14960 - Add @GeneratorType for better custom generator config
HHH-14718 - Drop deprecated generator implementations;
HHH-14959 - Drop IdentifierGeneratorFactory as a Service;
HHH-14960 - Add @GeneratorType for better custom generator config
@gavinking
Copy link
Member

Oh very nice. It's already done.

So, my #4427 (comment) WDYT, can it be done?

@yrodiere
Copy link
Member

yrodiere commented Dec 8, 2021

HHH-14959 - Drop IdentifierGeneratorFactory as a Service

@sebersole this means we urgently need a fix for HHH-14958, because until now overriding the IdentifierGeneratorFactory service was our only workaround. Do you intend to fix HHH-14958 soon? Otherwise, it might be better to keep the IdentifierGeneratorFactory service for now...

@yrodiere
Copy link
Member

yrodiere commented Dec 8, 2021

HHH-14959 - Drop IdentifierGeneratorFactory as a Service

@sebersole this means we urgently need a fix for HHH-14958, because until now overriding the IdentifierGeneratorFactory service was our only workaround. Do you intend to fix HHH-14958 soon? Otherwise, it might be better to keep the IdentifierGeneratorFactory service for now...

Also, even Quarkus needs that service to disable the "instantiate the generator through CDI" behavior. Though I'm not sure that's for the same reason. @Sanne might have more to say about this.

@sebersole
Copy link
Member Author

HHH-14959 - Drop IdentifierGeneratorFactory as a Service

@sebersole this means we urgently need a fix for HHH-14958, because until now overriding the IdentifierGeneratorFactory service was our only workaround. Do you intend to fix HHH-14958 soon? Otherwise, it might be better to keep the IdentifierGeneratorFactory service for now...

Have you tried it?

@sebersole
Copy link
Member Author

HHH-14959 - Drop IdentifierGeneratorFactory as a Service

@sebersole this means we urgently need a fix for HHH-14958, because until now overriding the IdentifierGeneratorFactory service was our only workaround. Do you intend to fix HHH-14958 soon? Otherwise, it might be better to keep the IdentifierGeneratorFactory service for now...

Also, even Quarkus needs that service to disable the "instantiate the generator through CDI" behavior. Though I'm not sure that's for the same reason. @Sanne might have more to say about this.

Yep, have you tried it? ;)

@sebersole
Copy link
Member Author

HHH-14959 - Drop IdentifierGeneratorFactory as a Service

@sebersole this means we urgently need a fix for HHH-14958, because until now overriding the IdentifierGeneratorFactory service was our only workaround. Do you intend to fix HHH-14958 soon? Otherwise, it might be better to keep the IdentifierGeneratorFactory service for now...

So the "problem" with keeping this a service is that it is kept around until SessionFactory closes while it is only used during this bootstrap phase. So it won't be made a service again, but if you really find it is still an issue (you have not tried and I do not think it is) we will provide a way for you to supply a custom one

HHH-14718 - Drop deprecated generator implementations;
HHH-14959 - Drop IdentifierGeneratorFactory as a Service;
HHH-14960 - Add @GeneratorType for better custom generator config

org.hibernate.id.factory.spi.StandardGenerator
HHH-14718 - Drop deprecated generator implementations;
HHH-14959 - Drop IdentifierGeneratorFactory as a Service;
HHH-14960 - Add @GeneratorType for better custom generator config

Fixed CustomGeneratorTests failure on databases which do not support sequences
@yrodiere
Copy link
Member

yrodiere commented Dec 8, 2021

HHH-14959 - Drop IdentifierGeneratorFactory as a Service

@sebersole this means we urgently need a fix for HHH-14958, because until now overriding the IdentifierGeneratorFactory service was our only workaround. Do you intend to fix HHH-14958 soon? Otherwise, it might be better to keep the IdentifierGeneratorFactory service for now...

Have you tried it?

No I haven't, I'm still trying to make 5.6 + Search 6 work in WildFly for now.

I'll interpret this as "Yes Yoann, I intend to fix this, in fact I already have". We'll know soon enough if there are still problems for ORM 6 in WildFly or Quarkus, I suppose.

@sebersole
Copy link
Member Author

I'll interpret this as "Yes Yoann, I intend to fix this, in fact I already have". We'll know soon enough if there are still problems for ORM 6 in WildFly or Quarkus, I suppose.

Yes, I think the flag you added could have just as easily been calculated - which is what I did.

If that does in fact not work, then fine - we will look at allowing you to plug in a custom factory (though not as a service)

@sebersole
Copy link
Member Author

I've pushed this work thus far upstream. We can tweak it from there

@sebersole sebersole closed this Dec 8, 2021
@sebersole sebersole deleted the topic/6-id-gen2 branch December 8, 2021 21:06
@gavinking
Copy link
Member

Excellent!

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