pulp / pulp_rpm

RPM support for Pulp Platform
https://docs.pulpproject.org/pulp_rpm/
GNU General Public License v2.0
48 stars 124 forks source link

Use new approach to binding fixtures #3468

Open pedro-psb opened 4 months ago

pedro-psb commented 4 months ago

I would like to have a similar bindings API as we have now in pulpcore (after this change).

As a developer, this approach feels more ergonomic. Other benefits should be equivalent to those discussed in the linked PR.

mdellweg commented 4 months ago

The only thing to hold you back is that this might be an unwarranted request to bump the pulpcore dependency.

pedro-psb commented 4 months ago

I'm not sure if I understand why it would cause an unwarranted bump request. Do you mean it would require pulpcore bumping because we would import BindingsNamespace (thus requiring a version where it exists)? We can do a try-catch and hardcode that class on exception, it looks like the code is simple enough for us to do that.

But anyway, any good plan to get around that?

mdellweg commented 4 months ago

I'm not sure if I understand why it would cause an unwarranted bump request. Do you mean it would require pulpcore bumping because we would import BindingsNamespace (thus requiring a version where it exists)? We can do a try-catch and hardcode that class on exception, it looks like the code is simple enough for us to do that.

But anyway, any good plan to get around that?

You nailed it. That's exactly what i was pointing at. Is it a good idea to require more recent pulpcore in order to use more recent testing features only? And maybe it is.

pedro-psb commented 4 months ago

Is it a good idea to require more recent pulpcore in order to use more recent testing features only?

I really can't tell if its a good idea or not. But to avoid the risk of any trouble this might cause, how about just copying BindingNamespace over? Assuming it is the only dependency required, it's a very simple one.

pedro-psb commented 1 month ago

We're already using the new pulpcore bindings fixtures after pulpcore 3.55 bump (here), so I guess its safe to proceed with this.