Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

DM-40053: Add Datastore records to DatasetRef and use them in butler.get #875

Merged
merged 12 commits into from
Oct 17, 2023
Merged

DM-40053: Add Datastore records to DatasetRef and use them in butler.get #875

merged 12 commits into from
Oct 17, 2023

Commits on Oct 17, 2023

  1. Add datastore records to DatasetRef

    @andy-slac
    andy-slac committed Oct 17, 2023
    Configuration menu
    Copy the full SHA
    ec466a6 View commit details
    Browse the repository at this point in the history

  2. Reimplement Butler.get with datastore records from registry. 

    There are few changes in Registry and Datastore classes that reduce use of
the bridge and opaque tables by datastores:
- New method `Datastore.get_opaque_table_definitions` returns opaque table
  definitoons which are used by Registry method `make_datastore_tables`
  to create those tables.
- `Registry.findDataset` has an option to retrieve datastore records
  and attach them to returned ref.
- `Datastore.get` can use the datastore records from the ref instead of
  using opaque tables.

Note that `Datastore.get` can still work with refs that don't have datastore
records, this is mostly to keep the unit tests running, a future commit will
remove that option and update all unit tests.
    @andy-slac
    andy-slac committed Oct 17, 2023
    Configuration menu
    Copy the full SHA
    3abade0 View commit details
    Browse the repository at this point in the history

  3. Re-implement Butler.put with datastore records saved by Registry. 

    Adds `Datastore.put_new`, we want to keep old `Datastore.put` to keep
unit tests running (no unit tests for `put_new()` yet). `put_new()`
returns DatasetRef with datastore records, and new Registry method
is called by Butler to store those records in database. Better
approach would be for Registry to store datastore records in
`insertDatasets`/`_importDatasets` but that needs a bit of code
re-arrangement, will do that later.
    @andy-slac
    andy-slac committed Oct 17, 2023
    Configuration menu
    Copy the full SHA
    8184c02 View commit details
    Browse the repository at this point in the history

  4. Remove dataset_id from StoredDatastoreItemInfo class. 

    This is to make it more similar to the `OpaqueTableValues` in DMTN-249
prototype. The `DatasetDatastoreRecords` type alias is similar to
`OpaqueTableBatch` from prototype. `StoredDatastoreItemInfo` is always
used with its corresponding `DatasetRef` or `DatasetId`, so keeping
dataset_id in each record is wasteful.
    @andy-slac
    andy-slac committed Oct 17, 2023
    Configuration menu
    Copy the full SHA
    4857945 View commit details
    Browse the repository at this point in the history

  5. Improve type checking fot generic datastore

    @andy-slac
    andy-slac committed Oct 17, 2023
    Configuration menu
    Copy the full SHA
    dbec173 View commit details
    Browse the repository at this point in the history

  6. Add do-nothing implementation of new methods to remote registry. 

    This is only to suppress errors in the tests.
    @andy-slac
    andy-slac committed Oct 17, 2023
    Configuration menu
    Copy the full SHA
    a75ee01 View commit details
    Browse the repository at this point in the history

  7. Rename OpaqueTableDefinition to DatastoreOpaqueTable 

    Old name was too generic, in reality this is very datastore-specific.
I do not think we need generic structure of the same kind, at least not
right now.
    @andy-slac
    andy-slac committed Oct 17, 2023
    Configuration menu
    Copy the full SHA
    9e125f9 View commit details
    Browse the repository at this point in the history

  8. Make datastore records private attribute of DatasetRef. 

    Jim suggested that we use new `OpaqueRecordSet` class to keep datastore
records, but after discussion we figured it would requre lots of additional
structure to support it. For now there is no clear need for that class and
we can continue using `StoredDatastoreItemInfo` in DatasetRefs, but want to
make records private to give us freedom to change it later.
    @andy-slac
    andy-slac committed Oct 17, 2023
    Configuration menu
    Copy the full SHA
    c4d1594 View commit details
    Browse the repository at this point in the history

  9. Make FileDatastore to ignore datastore records in some cases. 

    Datastore records in DatasetRef can become invalidated in some cases,
this is mostly true for our unit tests that do crazy things to test
consistency. This patch adds a flag parameter to some methods that
tells them to ignore ref-supplied datastore records.
    @andy-slac
    andy-slac committed Oct 17, 2023
    Configuration menu
    Copy the full SHA
    b63364f View commit details
    Browse the repository at this point in the history

  10. Revert implementation of Butler.put that used Datastore.put_new. 

    After trying to re-implement datastore unit tests I realized that the
`Datastore.put_new` and `Registry.store_datastore_records` combination
(used to implement `Butler.put`) is not very compatible with transaction
rollback system that we now have in place. So I decided to keep
`Butler.put` unchanged for now until we do something more drastic with
transactions. I keep `Datastore.put_new` and `Registry.store_datastore_records`
implementations, they are not used but may be useful in the future.
    @andy-slac
    andy-slac committed Oct 17, 2023
    Configuration menu
    Copy the full SHA
    19ac7b3 View commit details
    Browse the repository at this point in the history

  11. Use slots flag with datastore records dataclasses

    @andy-slac
    andy-slac committed Oct 17, 2023
    Configuration menu
    Copy the full SHA
    d147228 View commit details
    Browse the repository at this point in the history

  12. Apply review suggestions 

    Co-authored-by: Jim Bosch <[email protected]>
    @andy-slac @TallJimbo
    andy-slac and TallJimbo committed Oct 17, 2023
    Configuration menu
    Copy the full SHA
    2a1b9c1 View commit details
    Browse the repository at this point in the history