Christine Banek performed the code review in the normal way at https://github.com/lsst-sqre/lsst-tap-service/pull/33. The ticket review was meant to be a second-level review of the user experience. I'm happy to devise a more standardized procedure for this for the future.
It was my understanding (perhaps wrong) that there was no upstream Felis for the WISE and SDSS tables. However, apart from that, I have previously asked for us to think about where to source schema-level and table-level metadata, especially the schema_index and table_index values. It is not obvious to me that they belong in the Felis for the individual tables - I think a "curator" of the user experience needs to be able to choose the ordering of schemas and tables "after the fact", as it were, in the light of the entire ensemble of tables presented, and so that this information should not be frozen at the time the tables are originally defined. However, it is very clear that editing the same .sql files in which the column-level metadata appears is the wrong answer in the long run, as the latter should arise from Felis descriptions. So we need a workflow that merges information from per-table (Felis) sources and from one or more higher-level sources. Ideally that would be some sort of git-ops workflow that is triggered by pushing edits to the input files.
Gregory Dubois-Felsmann
added a comment - Colin Slater regarding your points:
Christine Banek performed the code review in the normal way at https://github.com/lsst-sqre/lsst-tap-service/pull/33 . The ticket review was meant to be a second-level review of the user experience. I'm happy to devise a more standardized procedure for this for the future.
It was my understanding (perhaps wrong) that there was no upstream Felis for the WISE and SDSS tables. However, apart from that, I have previously asked for us to think about where to source schema-level and table-level metadata, especially the schema_index and table_index values. It is not obvious to me that they belong in the Felis for the individual tables - I think a "curator" of the user experience needs to be able to choose the ordering of schemas and tables "after the fact", as it were, in the light of the entire ensemble of tables presented, and so that this information should not be frozen at the time the tables are originally defined. However, it is very clear that editing the same .sql files in which the column-level metadata appears is the wrong answer in the long run, as the latter should arise from Felis descriptions. So we need a workflow that merges information from per-table (Felis) sources and from one or more higher-level sources. Ideally that would be some sort of git-ops workflow that is triggered by pushing edits to the input files.
I'd like to close this ticket, and move further discussion elsewhere. Perhaps I should create a "Devise an architecture for the control of schema- and table-level metadata for the TAP service" ticket?
Gregory Dubois-Felsmann
added a comment - I'd like to close this ticket, and move further discussion elsewhere. Perhaps I should create a "Devise an architecture for the control of schema- and table-level metadata for the TAP service" ticket?
Ok, if Christine reviewed it then (in the future) you can set her as the reviewer and she can mark it as reviewed in Jira prior to merging as per the dev guide.
+1 on a new ticket for some architectural work, that sounds excellent to me.
Colin Slater
added a comment - Ok, if Christine reviewed it then (in the future) you can set her as the reviewer and she can mark it as reviewed in Jira prior to merging as per the dev guide.
+1 on a new ticket for some architectural work, that sounds excellent to me.
It was starting to sound familiar, and I recalled that DM-17472 already exists and covers the same area. I will add some comments there; it seems like we might not need a new ticket.
Gregory Dubois-Felsmann
added a comment - It was starting to sound familiar, and I recalled that DM-17472 already exists and covers the same area. I will add some comments there; it seems like we might not need a new ticket.
Colin Slater regarding your points: