Uploaded image for project: 'Request For Comments'
  1. Request For Comments
  2. RFC-22

remove old DB ingest capabilities prior to completion of new ones

    XMLWordPrintable

    Details

    • Type: RFC
    • Status: Implemented
    • Resolution: Done
    • Component/s: DM
    • Labels:
      None
    • Location:
      this issue page

      Description

      We're approaching the end of the conversion to the new measurement framework, and the last feature that needs to be implemented is database ingest. We're already working on that, but we have other work that's being blocked by the fact that we can't remove the old measurement framework until it's done. So I'm proposing here that we go ahead and remove the old measurement framework (and its associated old ingest scripts) now, resulting in a brief period in which we wouldn't have any ingest scripts at all.

      My understanding is that currently no one is using the old ingest scripts, unless they're doing so through an older tagged release, which wouldn't be affected, but I'd like to have that confirmed here, especially by the Jacek Becla and Dominique Boutigny.

      If this RFC is approved, we'd then proceed immediately with removing the old measurement framework in meas_algorithms, and removing both ap and datarel from the stack entirely, as currently I don't believe they provide any functionality besides ingest. At this point, I'm just proposing we remove them from buildbot and that the repos will be migrated to wherever completely dead repos go; the discussion on where exactly that should be should happen elsewhere.

        Attachments

          Issue Links

            Activity

            Hide
            ktl Kian-Tat Lim added a comment -

            What is being blocked? I'd kind of like to use this as practice for keeping something old running while something new is replacing it.

            Show
            ktl Kian-Tat Lim added a comment - What is being blocked? I'd kind of like to use this as practice for keeping something old running while something new is replacing it.
            Hide
            boutigny Dominique Boutigny added a comment -

            It was in my original plan to adapt whatever is necessary to ingest CFHT data into a mySQL DB, but I have found a way to run without the DB (at least for the short term) so I confirm that removing completly ap and datarel from the stack will not impact the work on CFHT. What about the schema in the cat package ?

            Show
            boutigny Dominique Boutigny added a comment - It was in my original plan to adapt whatever is necessary to ingest CFHT data into a mySQL DB, but I have found a way to run without the DB (at least for the short term) so I confirm that removing completly ap and datarel from the stack will not impact the work on CFHT. What about the schema in the cat package ?
            Hide
            jbosch Jim Bosch added a comment - - edited

            Kian-Tat Lim: What is being blocked?

            There are some simplifications to afw::table we can't undertake as long as the old measurement code is still using them, and some planned interface changes to the slots mechanism that would be a lot easier if we didn't have to support the old measurement code when we made them.

            We don't critically need to get those features in, but doing it this way would be the only we'd get them in during W15 (though admittedly even that might be a stretch, when review time is taken into account).

            Dominique Boutigny: What about the schema in the cat package ?

            I wondered about proposing that we remove cat too, but I figured the code in cat would not be broken by removal of the old measurement code (as the code in e.g. ap would), and I wasn't sure if the new ingest code was going to go into that package or a new one. Would love to get more information on that.

            Show
            jbosch Jim Bosch added a comment - - edited Kian-Tat Lim : What is being blocked? There are some simplifications to afw::table we can't undertake as long as the old measurement code is still using them, and some planned interface changes to the slots mechanism that would be a lot easier if we didn't have to support the old measurement code when we made them. We don't critically need to get those features in, but doing it this way would be the only we'd get them in during W15 (though admittedly even that might be a stretch, when review time is taken into account). Dominique Boutigny : What about the schema in the cat package ? I wondered about proposing that we remove cat too, but I figured the code in cat would not be broken by removal of the old measurement code (as the code in e.g. ap would), and I wasn't sure if the new ingest code was going to go into that package or a new one. Would love to get more information on that.
            Hide
            ktl Kian-Tat Lim added a comment -

            You can't have an A/B switch in afw::table?

            I'd like the new ingest code to go in a new package, formerly to be named daf_ingest. I was planning to have it be based on datarel/ingestSourcesTask.py. Note that that task "creates the destination table in the database if it doesn't exist", meaning that explicit SQL to pre-define the database tables would not be required (but may still be desirable for production purposes).

            Show
            ktl Kian-Tat Lim added a comment - You can't have an A/B switch in afw::table? I'd like the new ingest code to go in a new package, formerly to be named daf_ingest. I was planning to have it be based on datarel/ingestSourcesTask.py. Note that that task "creates the destination table in the database if it doesn't exist", meaning that explicit SQL to pre-define the database tables would not be required (but may still be desirable for production purposes).
            Hide
            jbosch Jim Bosch added a comment - - edited

            You can't have an A/B switch in afw::table?

            We have several in the form of if statements in several methods, all conditional on a "version" data member in afw::table::Schema. Is that what you had in mind? We can certainly keep those around longer - we just don't have much work left to do beyond getting the ingest going and removing all those conditionals, so I was hoping to do them in parallel. And, there is one item we really can't do this way (DM-1764, see also RFC-11), because we're actually removing all the code where the version specialization would go.

            Of course, just rejecting this RFC is something we can live with, as the penalty isn't terrible - we'll just start some S15 work early and delay the finish of some W15 work until the first S15 sprint (which, again, we'd likely have to do a bit of anyway).

            Show
            jbosch Jim Bosch added a comment - - edited You can't have an A/B switch in afw::table? We have several in the form of if statements in several methods, all conditional on a "version" data member in afw::table::Schema . Is that what you had in mind? We can certainly keep those around longer - we just don't have much work left to do beyond getting the ingest going and removing all those conditionals, so I was hoping to do them in parallel. And, there is one item we really can't do this way ( DM-1764 , see also RFC-11 ), because we're actually removing all the code where the version specialization would go. Of course, just rejecting this RFC is something we can live with, as the penalty isn't terrible - we'll just start some S15 work early and delay the finish of some W15 work until the first S15 sprint (which, again, we'd likely have to do a bit of anyway).
            Hide
            smonkewitz Serge Monkewitz added a comment - - edited

            I like daf_ingest as the location for the new code, and I was also planning to base it on K-T's ingestSources task. I think the main addition is that it will need to create views containing field aliases.

            Regarding temporarily not having db ingest capability: be advised that I am only 50% LSST and I'm not sure that I will have time to do anything about new ingest code until March.

            Regarding `cat`: there's stuff in cat that feels like it shouldn't be there at all (research/mysql?), python code that still uses pex_policy / pex_logging, custom code to interact with MySQL that should probably be replaced with lsst.db, plus various admin scripts that I don't know the state of (Jacek?). I don't think you can just remove it though, because it contains the baseline and data challenge schemas.

            Show
            smonkewitz Serge Monkewitz added a comment - - edited I like daf_ingest as the location for the new code, and I was also planning to base it on K-T's ingestSources task. I think the main addition is that it will need to create views containing field aliases. Regarding temporarily not having db ingest capability: be advised that I am only 50% LSST and I'm not sure that I will have time to do anything about new ingest code until March. Regarding `cat`: there's stuff in cat that feels like it shouldn't be there at all (research/mysql?), python code that still uses pex_policy / pex_logging, custom code to interact with MySQL that should probably be replaced with lsst.db, plus various admin scripts that I don't know the state of (Jacek?). I don't think you can just remove it though, because it contains the baseline and data challenge schemas.
            Hide
            ktl Kian-Tat Lim added a comment -

            You can "remove" methods like the getters based on version by throwing a (NotImplemented) exception if someone tries to use them. But that doesn't help if all the data generated by the new code is incompatible with the old ingest. So in some sense I think ingest is already dead, and we have to wait for Serge's availability in March to get it going again.

            Show
            ktl Kian-Tat Lim added a comment - You can "remove" methods like the getters based on version by throwing a (NotImplemented) exception if someone tries to use them. But that doesn't help if all the data generated by the new code is incompatible with the old ingest. So in some sense I think ingest is already dead, and we have to wait for Serge's availability in March to get it going again.
            Hide
            ktl Kian-Tat Lim added a comment -

            P.S. It's DM-1764 (and actually part of RFC-11), not DM-1746 that should be referenced above.

            Show
            ktl Kian-Tat Lim added a comment - P.S. It's DM-1764 (and actually part of RFC-11 ), not DM-1746 that should be referenced above.
            Hide
            jbosch Jim Bosch added a comment -

            I fixed the ticket link above to avoid future confusion.

            The sense in which I think ingest is alive is just that you can still switch back to the old measurement framework via config, and that should still produce outputs compatible with the old ingest. I don't think anyone has actually done that, though.

            Show
            jbosch Jim Bosch added a comment - I fixed the ticket link above to avoid future confusion. The sense in which I think ingest is alive is just that you can still switch back to the old measurement framework via config, and that should still produce outputs compatible with the old ingest. I don't think anyone has actually done that, though.
            Hide
            jbosch Jim Bosch added a comment -

            Kian-Tat Lim my sense is that you're still slightly uncomfortable with this, and at least at this stage, it's not looking as urgent as we'd originally thought (in that we still have some more work to before we can remove the old measurement framework anyway, so the work on ingest may catch up), so my inclination right now is to just Withdraw this RFC. If I've misread that, let me know.

            Show
            jbosch Jim Bosch added a comment - Kian-Tat Lim my sense is that you're still slightly uncomfortable with this, and at least at this stage, it's not looking as urgent as we'd originally thought (in that we still have some more work to before we can remove the old measurement framework anyway, so the work on ingest may catch up), so my inclination right now is to just Withdraw this RFC. If I've misread that, let me know.
            Hide
            ktl Kian-Tat Lim added a comment -

            No, I'm more uncomfortable that the ingest updates haven't been progressing along with the schema revisions, which has nothing to do with this RFC, really. But given that things are all up in the air right now (the prior-to-ingest transformation mechanisms being under review as well), if it makes your life easier to go ahead with this, I'm OK with it.

            Show
            ktl Kian-Tat Lim added a comment - No, I'm more uncomfortable that the ingest updates haven't been progressing along with the schema revisions, which has nothing to do with this RFC, really. But given that things are all up in the air right now (the prior-to-ingest transformation mechanisms being under review as well), if it makes your life easier to go ahead with this, I'm OK with it.
            Hide
            jbosch Jim Bosch added a comment -

            It may make our lives slightly easier down the road to have this flexibility, even if we don't need it just yet, and K-T has indicated that he's not opposed to it, so I'm accepting this.

            Show
            jbosch Jim Bosch added a comment - It may make our lives slightly easier down the road to have this flexibility, even if we don't need it just yet, and K-T has indicated that he's not opposed to it, so I'm accepting this.
            Hide
            jbosch Jim Bosch added a comment -

            Implemented on DM-2928.

            Show
            jbosch Jim Bosch added a comment - Implemented on DM-2928 .

              People

              Assignee:
              jbosch Jim Bosch
              Reporter:
              jbosch Jim Bosch
              Watchers:
              Dominique Boutigny, Jim Bosch, John Swinbank, Kian-Tat Lim, Serge Monkewitz
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:
                Planned End:

                  Jenkins

                  No builds found.