Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-20842

Move FileDescriptor to a property in Formatter

    XMLWordPrintable

Details

    • Story
    • Status: Done
    • Resolution: Done
    • None
    • daf_butler
    • None

    Description

      Discussing DM-20763 with Parejkoj leads me to think that datastore should guarantee that Formatter instances are not reused. This would allow formatter implementations to cache information with the knowledge that they don't have to worry about the cache being wrong if the file descriptor changes. To make this more obvious I'm going to investigate changing Formatter so that FileDescriptor is a mandatory constructor argument becoming a read-only property. This will make it impossible for the formatter to be used for any other file and will mean that the argument is removed from the read and write methods.

      Attachments

        Issue Links

          Activity

            No builds found.
            tjenness Tim Jenness created issue -
            tjenness Tim Jenness made changes -
            Field Original Value New Value
            Epic Link DM-20101 [ 319087 ]
            tjenness Tim Jenness made changes -
            Link This issue relates to DM-20763 [ DM-20763 ]
            Parejkoj John Parejko added a comment - - edited

            You'll have to also update HyperSuprimeCamRawFormatter.readImage() and obs.base.FitsRawFormatterBase on this PR. I think you can just cherry-pick my commits from DM-20763.

            Parejkoj John Parejko added a comment - - edited You'll have to also update HyperSuprimeCamRawFormatter.readImage() and obs.base.FitsRawFormatterBase on this PR. I think you can just cherry-pick my commits from DM-20763 .
            tjenness Tim Jenness added a comment -

            I know that I have more to do. My concern was that this is such a major change to the formatter API that I wanted you and jbosch to sign off before I proceeded to make all the other packages work.

            tjenness Tim Jenness added a comment - I know that I have more to do. My concern was that this is such a major change to the formatter API that I wanted you and jbosch to sign off before I proceeded to make all the other packages work.
            tjenness Tim Jenness added a comment -

            Given that jbosch has signed off on the PR in daf_butler I'll now work on getting obs_subaru etc fixed up.

            tjenness Tim Jenness added a comment - Given that jbosch has signed off on the PR in daf_butler I'll now work on getting obs_subaru etc fixed up.
            tjenness Tim Jenness added a comment -

            I've cleaned this up and I think it does look better. At jbosch's request I have allowed FormatterFactory to retrieve the class as well as an instance and that has made things more explicit. I think we should probably change Instrument.getRawFormatter() to return a class rather than an instance now but I think that can be done as part of DM-20763.

            tjenness Tim Jenness added a comment - I've cleaned this up and I think it does look better. At jbosch 's request I have allowed FormatterFactory to retrieve the class as well as an instance and that has made things more explicit. I think we should probably change Instrument.getRawFormatter() to return a class rather than an instance now but I think that can be done as part of DM-20763 .
            tjenness Tim Jenness made changes -
            Reviewers Jim Bosch, John Parejko [ jbosch, parejkoj ]
            Status To Do [ 10001 ] In Review [ 10004 ]
            tjenness Tim Jenness made changes -
            Epic Link DM-20101 [ 319087 ] DM-20102 [ 319090 ]
            tjenness Tim Jenness made changes -
            Story Points 4
            jbosch Jim Bosch added a comment -

            Everything looks good to me.  Just a few extremely minor comments on the daf_butler PR.  Leaving "In Review" only because there are two of us.

             

            jbosch Jim Bosch added a comment - Everything looks good to me.  Just a few extremely minor comments on the daf_butler PR.  Leaving "In Review" only because there are two of us.  

            A couple comments on the PRs, but this looks good. Thanks for doing it.

            Parejkoj John Parejko added a comment - A couple comments on the PRs, but this looks good. Thanks for doing it.
            Parejkoj John Parejko made changes -
            Status In Review [ 10004 ] In Review [ 10004 ]
            Parejkoj John Parejko made changes -
            Status In Review [ 10004 ] Reviewed [ 10101 ]
            tjenness Tim Jenness made changes -
            Resolution Done [ 10000 ]
            Status Reviewed [ 10101 ] Done [ 10002 ]

            People

              tjenness Tim Jenness
              tjenness Tim Jenness
              Jim Bosch, John Parejko
              Jim Bosch, John Parejko, Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Jenkins

                  No builds found.