ska3-perl for shiny release candidate #508
Conversation
|
I don't have time to scrutinize the code now, but can you please provide a proper description for this PR of the rationale for the change and the intended use case. |
|
Little confused by what is a proper description given context? I just thought we wanted a ska3-perl metapackage with a set of package versions to go into shiny... but sure. |
jeanconn
changed the title
|
This is a tiny PR against 2020.13+shiny branch. I was originally going to push these two tiny commits directly to 2020.13+shiny branch, but figured that it would be a bit rude without letting Javier have a look. In 2020.13+shiny there's a new combine_arch.py to make ska3-core. That script has logic to take the package versions from two platforms, subtract ska packages, and make one list of everything left over. We also have need for a platform-specific all-versioned package for ska3-perl made from the "extra" packages installed in ska3 by the ska3-perl-latest package, so some of the combine_arch code seemed useful. It seemed to me that the easiest way to do that was to make an environment with ska3-core ska3-flight ska3-perl-latest, and subtract off all the packages already defined in an environment created just by ska3-core ska3-flight. so I added code to the modified combine_arch.py helper script to remove packages on a platform from a separate json. For this PR, the question is really: should these changes end up in combine_arch.py or should I just toss it into a gist like Javier's at https://gist.github.com/javierggt/f64fd5300c1e946e9f3f52f483195e8c ? They probably don't belong in combine_arch if we leave that script hanging around in the ska3-core directory. It just seemed to me that it was becoming a bit more general. I've changed this to a WIP because I haven't tested both ska3-perl packages. |
jeanconn
changed the title
|
I confirmed the ska3-perl packages build and look to work so I just removed the WIP. |
|
@taldcroft, the bottomline is this:
I suggested just listing packages installed by ska3-perl-latest (without removing packages in ska3-core) unless we make ska3-core a requirement of ska3-perl. For example, if you create a fresh environment and install ska3-core-latest, you will have yaml, sqlite, numpy and others. If you install just ska3-perl you might not get those, because they come through ska3-core and ska3-core is not a requirement. |
|
If we aren't going to be making new ska3-core packages really often, I don't care, so I just added ska3-core to the draft ska3-perl metapackage. I also don't know how much we need to refine process for updating the all-versioned metapackages. So I don't know what we want to do with this PR. I was anticipating a small amount of review and refinement from @javierggt and then a merge into the development branch with the idea that @taldcroft will be reviewing the bigger PR on the 2020.13+shiny branch next week. |
There was a problem hiding this comment.
In general this looks OK and the meta.yaml looks reasonable.
My real comment which is more substantial is that we need the process of generating this file to be documented and reproducible. In this case Jean needed to think about things and then make two more commits. We will go through this whole exercise again in maybe 2 years by which time all of this will be forgotten entirely (by me anyway). So no hand-edits, or if there are then they need to be carefully documented.
To that end, one path forward is:
- Move watch_cron_logs and task_schedule into ska3-flight. Honestly I really don't care that they get installed and won't run. There is nobody that will try to do this and get confused because they don't run. The user community for those packages is us three.
- Remove the dependence on ska3-core here. Indeed nothing will work without ska3-core. Is there any single user that will get tripped up by this? No.
What I am saying here is to emphasize process simplicity over the perfection of having all the dependencies correct for mix-n-match installation of different components. This latter feature of conda is not a requirement for Ska3 meta packages now and I haven't yet heard a compelling reason to support this (given the real cost of doing so).
With that I think of this meta package as being essentially a plug-in to Ska3. Of course it won't work if you don't have ska3-core installed, but do we care? My feeling is no.
If we take this approach then I think (??) that the current combine_arch.py will generate the correct file out of the box and we're done and having something simple to document/maintain and reproducible.
|
Awesome. Thanks for the feedback Tom!
Agree 100%. I have notes but was planning to finalize them after reusing whatever we decided as a final script to remake the meta.yaml in a process that would end up at the bottom of the shiny twiki page with Javier's notes.
Sure. That's a path forward. I think it is probably more trivial to update the script to make ska3-perl to include them correctly? But if you think it makes more sense for them to live in ska3-flight that's fine. I know we'd gone back and forth on them. I suppose I'd just need to start by cutting new versions of them after updating their conda recipes to remove all their dependencies. Will test that out.
Understood. Though I did think that a good point that came up for me from the discussion with Javier is that if we update ska3-core, we probably need to test and cut a new version of ska3-perl anyway (because changes in the core packages can conflict or change what ends up in ska3-perl made from ska3-perl-latest). Because of that, I don't feel strongly that keeping ska3-core out of ska3-perl saves us much (still need to do the process on an update), but also fine to strip it out again.
Well, technically the script now puts out something called ska3-core at the top (and lives in the ska3-core directory), so it either needs a few additional tiny tweaks or documented steps... but will get the ducks in a row. |
|
I guess we understand "simplicity" differently. What I suggested is entirely reproducible in an automated way, and required:
Leaving out ska3-core from the dependencies appears to decouple the two packages so you can update one and not the other, but in reality there will be implicit dependencies, so ska3-perl might need updating whenever ska3-core is updated anyway. Explicit is better than implicit, and it does not increase complexity significantly. I don't know why Jean had to manually add packages in the end. Were they not already in the input environments? |
I was pretty sure they got excluded because they were ska packages and the current ska3-core/combine_arch is set to exclude ska packages via the skare3_tools packages mechanism. So the simplest thing would be to modify this combine_arch draft script to not exclude packages via that mechanism when a subtracting off an environment supplied by the subtract-env option (it shouldn't do both). I'm also still not sure if the best plan is to make combine_arch perfect or just exactly capture what we did (like how to make ska3-flight) in a separate script / gist. |
|
About the ska3-core, I got misled by the commit where |
|
I don't think we are *all* the way there yet with this script and the
interaction between ska-perl and ska3-core? But I'm on-board with the
though that it doesn't matter if ska3-core is in the ska3-perl metapackage
as long as we have a reproducible process with very few or no manual edits.
|
Makes sense, and I would agree with that. Strictly speaking, we could do away with the skare3_tools.packages option and create ska3-core by installing ska3-flight-latest and subtracting ska3-core-latest. This means the procedure would be the same for both ska3-core and ska3-perl, and all the arbitrariness is absorbed in the creation of ska3-*-latest. Then, if you add ska3-core-latest to the dependencies of ska3-perl-latest, you get a ska3-core dependency in the end. |
|
I haven't checked, will your version RC rewriting stuff work to update the ska3-core version as a dependency? |
|
I suppose it is set to do that in ska3-flight already but wasn't sure where that was configured so it would work in a new metapackage like ska3-perl. |
no, but it would be an easy addition. I would add something that replaces all appearances of
|
|
I'm still not sure that ska3-core in ska3-perl wins us much. We'd add ska3-core-latest to ska3-perl-latest for testing, but would still need a manual step (or exception/special case in the code) to add ska3-core-latest (or ska3-core at a version to be magically updated) to ska3-perl. |
|
actually... ska3-core would appear in both the ska3-flight and ska3-core environments, so by subtracting them it would not be in the final ska3-flight env. But still, one can add exceptions to |
|
My suggestion is to merge this, and I will add the option to handle We all agree the process should be straight-forward and reproducible, and it is going to be faster to implement it than to discuss it. Then we can discuss based on the implementation. |
|
Sure. I'll just clean up that one part that Tom called out as DRY . Then we're left with
|
|
I just did that DRY one. |
|
See 34bdad0. |
|
Sorry I should have just left this alone and let you do it. |
|
Though I will argue my refactor is cleaner. |
|
Haha. I was just going to tell you yours was one step cleaner, but you already took credit... |
|
So you should practice DRY on every script, even ones that you consider to be low-grade utility scripts. |
Make a ska3-perl versioned metapackage for the shiny release candidate.
Update combine_arch helper/utility script to with an option to subtract off the packages described by another platform-specific json file. This new option was used to make the ska3-perl metapackage as the ska3-perl package should basically be the difference between the environment defined by (ska3-core ska3-flight ska3-perl-latest) and the one defined by (ska3-core ska3-flight).