search

AcademySoftwareFoundation / rez

An integrated package configuration, build and deployment system for software
https://rez.readthedocs.io
Apache License 2.0
949 stars 338 forks source link

per-package-family package orderers needed #403

Open nerdvegas opened 7 years ago

nerdvegas commented 7 years ago

I have a case where I want to use the TimestampPackageOrder class, but I need different orders depending on the package family.

One approach would be to add an optional packagename arg to this orderer, but that's unoptimal - to use 10 different oderers over 100 different packages, we'd have to iterate over the 10 orderers for every package.

Instead I think it makes sense to add a new orderer, PerFamilyOrder, where you can associate other Orderers with separate families (and be able to specify a default for all packages that don't have a specific orderer applied). This means applying to each package only involves a dict lookup - but more specifically, this is probably a common pattern, that this orderer will now allow us to apply to many situations involving different orderers.

pmolodo commented 7 years ago

Hi allan - have you taken a look at my pull request for custom package orderers? (https://github.com/nerdvegas/rez/pull/355/commits)

The first commit addresses configuring of package orderers, and allows you to set them per-package-family. You don't have to merge the rest of the commits if you don't want to yet, but the first at least should address your particular concern here.

On Wed, Mar 22, 2017 at 4:14 PM allan johns notifications@github.com wrote:

I have a case where I want to use the TimestampPackageOrder class, but I need different orders depending on the package family.

One approach would be to add an optional packagename arg to this orderer, but that's unoptimal - to use 10 different oderers over 100 different packages, we'd have to iterate over the 10 orderers for every package.

Instead I think it makes sense to add a new orderer, PerFamilyOrder, where you can associate other Orderers with separate families (and be able to specify a default for all packages that don't have a specific orderer applied). This means applying to each package only involves a dict lookup - but more specifically, this is probably a common pattern, that this orderer will now allow us to apply to many situations involving different orderers.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/nerdvegas/rez/issues/403, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEYeL9ay1I3zTvm-kxCiYpPDR4wwvQFks5roatJgaJpZM4Ml8UM .

pmolodo commented 7 years ago

ie, it would allow you to do something like this:

package_orderers:

​

On Thu, Mar 23, 2017 at 9:20 AM Paul Molodowitch elrond79@gmail.com wrote:

Hi allan - have you taken a look at my pull request for custom package orderers? (https://github.com/nerdvegas/rez/pull/355/commits)

The first commit addresses configuring of package orderers, and allows you to set them per-package-family. You don't have to merge the rest of the commits if you don't want to yet, but the first at least should address your particular concern here.

On Wed, Mar 22, 2017 at 4:14 PM allan johns notifications@github.com wrote:

I have a case where I want to use the TimestampPackageOrder class, but I need different orders depending on the package family.

One approach would be to add an optional packagename arg to this orderer, but that's unoptimal - to use 10 different oderers over 100 different packages, we'd have to iterate over the 10 orderers for every package.

Instead I think it makes sense to add a new orderer, PerFamilyOrder, where you can associate other Orderers with separate families (and be able to specify a default for all packages that don't have a specific orderer applied). This means applying to each package only involves a dict lookup - but more specifically, this is probably a common pattern, that this orderer will now allow us to apply to many situations involving different orderers.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/nerdvegas/rez/issues/403, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEYeL9ay1I3zTvm-kxCiYpPDR4wwvQFks5roatJgaJpZM4Ml8UM .

pmolodo commented 7 years ago

Hmm, just read your note about efficiency - which this wouldn't address. It would be pretty easy to modify the syntax so it took a dict OR a list - if a dict, it would map from package families to a list of orderers, purely for efficiency. I still feel this makes more sense than having a specific type of orderer just to solve what's essentially a configuration issue

On Thu, Mar 23, 2017 at 9:24 AM Paul Molodowitch elrond79@gmail.com wrote:

ie, it would allow you to do something like this:

package_orderers:

  • type: soft_timestamp packages: ["foo", "bar"] timestamp: 1429830188
  • type: soft_timestamp packages: "*" timestamp: 1429862387 other_settings_for_soft_timestamp: ['blah']

​

On Thu, Mar 23, 2017 at 9:20 AM Paul Molodowitch elrond79@gmail.com wrote:

Hi allan - have you taken a look at my pull request for custom package orderers? (https://github.com/nerdvegas/rez/pull/355/commits)

The first commit addresses configuring of package orderers, and allows you to set them per-package-family. You don't have to merge the rest of the commits if you don't want to yet, but the first at least should address your particular concern here.

On Wed, Mar 22, 2017 at 4:14 PM allan johns notifications@github.com wrote:

I have a case where I want to use the TimestampPackageOrder class, but I need different orders depending on the package family.

One approach would be to add an optional packagename arg to this orderer, but that's unoptimal - to use 10 different oderers over 100 different packages, we'd have to iterate over the 10 orderers for every package.

Instead I think it makes sense to add a new orderer, PerFamilyOrder, where you can associate other Orderers with separate families (and be able to specify a default for all packages that don't have a specific orderer applied). This means applying to each package only involves a dict lookup - but more specifically, this is probably a common pattern, that this orderer will now allow us to apply to many situations involving different orderers.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/nerdvegas/rez/issues/403, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEYeL9ay1I3zTvm-kxCiYpPDR4wwvQFks5roatJgaJpZM4Ml8UM .

nerdvegas commented 7 years ago

Ah crap, I've just gone and implemented a per-family orderer, that's why I put that issue in. This is for an unrelated reason. Sorry about that.

My orderer just lets you specify other orderers on a per-family basis, and you can also set a default orderer. Looks the same as you've done but addresses the efficiency issue.

I was about to merge this (I've added other orderers too) but I think now is the right time to take your own PR and include it in this release, so I'll do that.

Wrt doing this via config vs orderer: I hear you RE doing it in config, but that doesn't help when I just want to create an orderer via API. I think it's also a better abstraction because it doesn't affect any of the rest of the code - the Solver still just takes a list of orderers. It's more flexible too, because I may want to extend the PerFamilyOrderer (or implement another Orderer) to work based on, say, package attributes, rather than explicit package names (eg, apply orderer X when package Y meets condition 'package.package_type == 'maya'). If I do that via config then I have to add more code to the Solver to take these configuration issues into account. This example is actually a realistic case, I'm expecting there's a good chance I'll implement it in order to address my studio's needs.

Your mail raised an issue with my orderer though - I can only specify an orderer per package family, not a list of families. I'll address that, and merge what we both have.

Thanks A

On Fri, Mar 24, 2017 at 3:27 AM, Paul Molodowitch notifications@github.com wrote:

Hmm, just read your note about efficiency - which this wouldn't address. It would be pretty easy to modify the syntax so it took a dict OR a list - if a dict, it would map from package families to a list of orderers, purely for efficiency. I still feel this makes more sense than having a specific type of orderer just to solve what's essentially a configuration issue

On Thu, Mar 23, 2017 at 9:24 AM Paul Molodowitch elrond79@gmail.com wrote:

ie, it would allow you to do something like this:

package_orderers:

  • type: soft_timestamp packages: ["foo", "bar"] timestamp: 1429830188
  • type: soft_timestamp packages: "*" timestamp: 1429862387 other_settings_for_soft_timestamp: ['blah']

​

On Thu, Mar 23, 2017 at 9:20 AM Paul Molodowitch elrond79@gmail.com wrote:

Hi allan - have you taken a look at my pull request for custom package orderers? (https://github.com/nerdvegas/rez/pull/355/commits)

The first commit addresses configuring of package orderers, and allows you to set them per-package-family. You don't have to merge the rest of the commits if you don't want to yet, but the first at least should address your particular concern here.

On Wed, Mar 22, 2017 at 4:14 PM allan johns notifications@github.com wrote:

I have a case where I want to use the TimestampPackageOrder class, but I need different orders depending on the package family.

One approach would be to add an optional packagename arg to this orderer, but that's unoptimal - to use 10 different oderers over 100 different packages, we'd have to iterate over the 10 orderers for every package.

Instead I think it makes sense to add a new orderer, PerFamilyOrder, where you can associate other Orderers with separate families (and be able to specify a default for all packages that don't have a specific orderer applied). This means applying to each package only involves a dict lookup - but more specifically, this is probably a common pattern, that this orderer will now allow us to apply to many situations involving different orderers.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/nerdvegas/rez/issues/403, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEYeL9ay1I3zTvm- kxCiYpPDR4wwvQFks5roatJgaJpZM4Ml8UM .

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/nerdvegas/rez/issues/403#issuecomment-288777010, or mute the thread https://github.com/notifications/unsubscribe-auth/ABjqSnKSyk6CKexQf_3NWmcRUq6RWHXrks5rop1vgaJpZM4Ml8UM .

pmolodo commented 7 years ago

Ok... as long as we preserve the ability for each orderer type to determine it's own rules for whether it applies to a given family. Otherwise the syntax for defining a CustomPacakgeOrderer for more than a few packages would get cumbersome.