google / ml-metadata

For recording and retrieving metadata associated with ML developer and data scientist workflows.
https://www.tensorflow.org/tfx/guide/mlmd
Apache License 2.0
616 stars 145 forks source link

Provide sqlite configs as cmdline flags #189

Open lampajr opened 9 months ago

lampajr commented 9 months ago

While using ml-metadata I noticed that there is no way to startup the server using sqlite configuration through flags, the only way I found (as stated by the documentation) was to use the config_file file option pointing to a pb file like:

connection_config {
  sqlite {
    filename_uri: '/tmp/test_db'
    connection_mode: READWRITE_OPENCREATE
  }
}

With this pull request I am proposing to introduce the possibility to startup the grpc server with sqlite config using cmdline flags, similarly to what is currently supported for mysql and postgresql.

Given this, the previous configuration could be replaced by cmdline flags as follow:

docker run -p ${MLMD_GRPC_PORT}:${MLMD_GRPC_PORT} --entrypoint /bin/metadata_store_server "${MLMD_DOCKER_IMAGE}" \
--grpc_port=${MLMD_GRPC_PORT} \
--metadata_source_config_type="sqlite" \
--sqlite_config_filename_uri=/tmp/test_db \
--sqlite_config_connection_mode=READWRITE_OPENCREATE

Technical addition

This PR proposes to add a:

isinyaaa commented 9 months ago

Nit: you might want to update this comment: https://github.com/google/ml-metadata/pull/189/files#diff-0c3fe2859affc74bb85407ef9414003f0f77e4d5a0db30d3f67e57b03c5979ebR344

Edit: this error message must also be updated: https://github.com/google/ml-metadata/pull/189/files#diff-0c3fe2859affc74bb85407ef9414003f0f77e4d5a0db30d3f67e57b03c5979ebR340

lampajr commented 9 months ago

Nit: you might want to update this comment: https://github.com/google/ml-metadata/pull/189/files#diff-0c3fe2859affc74bb85407ef9414003f0f77e4d5a0db30d3f67e57b03c5979ebR344

Edit: this error message must also be updated: https://github.com/google/ml-metadata/pull/189/files#diff-0c3fe2859affc74bb85407ef9414003f0f77e4d5a0db30d3f67e57b03c5979ebR340

Nice catch, updated!

lampajr commented 8 months ago

Hey @XinranTang, is there any chance to get this in?

lampajr commented 5 months ago

Hey @XinranTang, is there any chance to get this in?

Any news?