Adding database information to v.info metadata output

[huhabla]

I would like to extent the metadata output of v.info with information about the database used to store the attribute data. I have implemented several tests to assure correct implementation.

Additional output is:

attribute_layer_number=1
attribute_layer_name=test_vinfo_with_db_3d
attribute_database=/home/user/g/LL/user/vector/test_vinfo_with_db_3d/sqlite.db
attribute_database_driver=sqlite
attribute_table=test_vinfo_with_db_3d
attribute_primary_key=cat

The patch is attached.

Unfortunately my editor changed some empty lines that got mixed up with the patch.

[huhabla]

Patch to enable database metadata and unit-tests for v.info

[martinl]

Please go ahead with applying the patch in trunk and relbr72. Ideally without empty lines.

[mlennert]

Why do you need this in v.info and what's wrong with v.db.connect -p ?

[huhabla]

Replying to mlennert:

Why do you need this in v.info and what's wrong with v.db.connect -p ?

Why is this information not present in v.info? Why do we need a second command to get this information about a vector map? The output of v.db.connect -g is not well designed for parsing, neither for bash nor for the python-grass-script library.

[mlennert]

Replying to huhabla:

Replying to mlennert:

Why do you need this in v.info and what's wrong with v.db.connect -p ?

Why is this information not present in v.info? Why do we need a second command to get this information about a vector map?

Because it's module that manages that setting that provides the info, as with other modules (e.g. v.category).

The output of v.db.connect -g is not well designed for parsing, neither for bash nor for the python-grass-script library.

Then let us improve that :-)

Just to be clear: this is not a -1 to your idea. I just want to make sure we think about it. In general, I'm wary of introducing the same functionality in two different modules. IMHO, it creates the risk of feature bloat and of confusion for the users.

[huhabla]

Replying to mlennert:

Replying to huhabla:

Replying to mlennert:

Why do you need this in v.info and what's wrong with v.db.connect -p ?

Why is this information not present in v.info? Why do we need a second command to get this information about a vector map?

Because it's module that manages that setting that provides the info, as with other modules (e.g. v.category).

Exactly, it manages the database connection and should also display the connection parameter.

The output of v.db.connect -g is not well designed for parsing, neither for bash nor for the python-grass-script library.

Then let us improve that :-)

I guess there was a specific reason behind the implementation of the "-g" flag in v.db.connect in the way it is. I fear to break existing functionality when modifying v.db.connect "-g" flag output. Since we do not have enough module tests, it would be hard to figure out what will break in existing modules, in examples and documented work flows and even more harder to figure out if extensions will break.

Just to be clear: this is not a -1 to your idea. I just want to make sure we think about it. In general, I'm wary of introducing the same functionality in two different modules. IMHO, it creates the risk of feature bloat and of confusion for the users.

IMHO the database connection is vector map metadata that should be available within v.info, so the user does not have to search for another module to get this information. The default settings of vector layer database management usually does not require the user to use v.db.connect, hence its capabilities is mostly unknown to the user.

I think the less invasive solution is to modify v.info, especially since i have already implemented 6 tests to verify the functionality. ;)

BTW, v.info is an integral part of the gunittest validation approach, hence the more metadata information about a vector map is available via v.info, the easier it is to implement tests.

[mlennert]

Replying to huhabla:

Replying to mlennert:

Replying to huhabla:

The output of v.db.connect -g is not well designed for parsing, neither for bash nor for the python-grass-script library.

Then let us improve that :-)

I guess there was a specific reason behind the implementation of the "-g" flag in v.db.connect in the way it is. I fear to break existing functionality when modifying v.db.connect "-g" flag output. Since we do not have enough module tests, it would be hard to figure out what will break in existing modules, in examples and documented work flows and even more harder to figure out if extensions will break.

One option could be to create another flag (-f?) which would "unFlatten" the output of -f and provide a key=value output.

Just to be clear: this is not a -1 to your idea. I just want to make sure we think about it. In general, I'm wary of introducing the same functionality in two different modules. IMHO, it creates the risk of feature bloat and of confusion for the users.

IMHO the database connection is vector map metadata that should be available within v.info, so the user does not have to search for another module to get this information. The default settings of vector layer database management usually does not require the user to use v.db.connect, hence its capabilities is mostly unknown to the user.

I understand the concern, but remain afraid that such logic leads to feature bloat of modules as many things would be nice to have in one single module. But then again, I'm probably just biased as I started using GRASS before the advent of v.db.*...

I think the less invasive solution is to modify v.info, especially since i have already implemented 6 tests to verify the functionality. ;)

As much as I understand that already done work would be frustrating to abandon, I don't think that this should enter into account in this discussion...

BTW, v.info is an integral part of the gunittest validation approach, hence the more metadata information about a vector map is available via v.info, the easier it is to implement tests.

Because otherwise you have to call two modules ? Again, I would not want test creation logic to be what decides about module functionality.

But I'll shut up now and let you go on with your work. As others don't seem to share my worries, this is probably me just rambling on a hangover Sunday... ;-)

[huhabla]

Replying to mlennert:

Replying to huhabla:

Replying to mlennert:

Replying to huhabla:

The output of v.db.connect -g is not well designed for parsing, neither for bash nor for the python-grass-script library.

Then let us improve that :-)

I guess there was a specific reason behind the implementation of the "-g" flag in v.db.connect in the way it is. I fear to break existing functionality when modifying v.db.connect "-g" flag output. Since we do not have enough module tests, it would be hard to figure out what will break in existing modules, in examples and documented work flows and even more harder to figure out if extensions will break.

One option could be to create another flag (-f?) which would "unFlatten" the output of -f and provide a key=value output.

The "-g" flag is used for key-value shell output in many modules, adding a new flag to generate shell output is counter intuitive and makes the code more complicated.

Just to be clear: this is not a -1 to your idea. I just want to make sure we think about it. In general, I'm wary of introducing the same functionality in two different modules. IMHO, it creates the risk of feature bloat and of confusion for the users.

IMHO the database connection is vector map metadata that should be available within v.info, so the user does not have to search for another module to get this information. The default settings of vector layer database management usually does not require the user to use v.db.connect, hence its capabilities is mostly unknown to the user.

I understand the concern, but remain afraid that such logic leads to feature bloat of modules as many things would be nice to have in one single module. But then again, I'm probably just biased as I started using GRASS before the advent of v.db.*...

I agree, adding unnecessary flags to modules will lead to feature bloat. And i agree that a single module to access information (for example metadata) without adding additional flags is preferable.

If i would need to add new flags or options to v.info to display the database connection metadata, i wouldn't implement it. The suggested implementation simply adds the database connection info to the -e flag the "extended metadata" of v.info. Only some lines of code are added, no new feature, no highly redundant code, no esoteric information.

I think the less invasive solution is to modify v.info, especially since i have already implemented 6 tests to verify the functionality. ;)

As much as I understand that already done work would be frustrating to abandon, I don't think that this should enter into account in this discussion...

Indeed.

BTW, v.info is an integral part of the gunittest validation approach, hence the more metadata information about a vector map is available via v.info, the easier it is to implement tests.

Because otherwise you have to call two modules ? Again, I would not want test creation logic to be what decides about module functionality.

Why call two modules to get the information that a single module should provide? And why shouldn't test logic be used to define module functionality? How about if the test logic, which is in our case dedicated to test GRASS functionality, exposes inconsistent logic or inconsistent output definitions in modules? Should we ignore this, because it was found by test logic? In this specific case i realized, that the database information is missing in v.info and that tests can't be implemented to check for database connection parameters easily. Our GRASS gunittest environment should not depend on to many modules to check correct module or API test runs. This would be bad design, to many points of failure. Hence, test logic will influence module design.

But I'll shut up now and let you go on with your work. As others don't seem to share my worries, this is probably me just rambling on a hangover Sunday... ;-)

Don't worry, it is always good to argue about specific changes. We all have different opinions and biases about implementation and user experience. Its good to exchange arguments to come to a consent. I will wait for more concerns before i apply the patch.

[mmetz]

Replying to huhabla:

Replying to mlennert:

Replying to huhabla:

The output of v.db.connect -g is not well designed for parsing, neither for bash nor for the python-grass-script library.

Then let us improve that :-)

I guess there was a specific reason behind the implementation of the "-g" flag in v.db.connect in the way it is. I fear to break existing functionality when modifying v.db.connect "-g" flag output. Since we do not have enough module tests, it would be hard to figure out what will break in existing modules, in examples and documented work flows and even more harder to figure out if extensions will break.

The output of v.db.connect -g prints all connections for all layers, therefore the typical key=value approach does not work well, instead you need to tokenize each output line. Also, the python grass script library uses the output of v.db.connect -g e.g. in vector_db which works just fine, also for multiple connections.

IMHO the database connection is vector map metadata that should be available within v.info, so the user does not have to search for another module to get this information. The default settings of vector layer database management usually does not require the user to use v.db.connect, hence its capabilities is mostly unknown to the user.

I think the less invasive solution is to modify v.info, especially since i have already implemented 6 tests to verify the functionality. ;)

Please make sure that your patch for v.info only prints connection parameters if a single, valid layer is selected because your approach can not handle multiple connections.

[wenzeslaus]

Replying to mmetz:

...your approach can not handle multiple connections.

This is a common problem which was not yet addressed well. One solution is to use something like this:

db_linked_layers=1,2
attribute_layer_1_layer_name=test_vinfo_with_db_3d
attribute_layer_1_database=/home/user/g/LL/user/vector/test_vinfo_with_db_3d/sqlite.db
attribute_layer_1_database_driver=sqlite
...
attribute_layer_2_layer_name=...

The naming should be improved but the idea is similar to OSM tagging:

emergency=fire_hydrant
fire_hydrant:type=underground
fire_hydrant:diameter=1
...

Alternative and more universal approach is using JSON as in case of G7:v.what. Something like this:

...
"dblinks": [
    {
        "number": 1,
        "name": "test_vinfo_with_db_3d",
        "database": "/home/user/g/LL/user/vector/test_vinfo_with_db...",
        ...
    {
]
...

Also compare:

g.extension -g
g.search.modules gradient -j

[neteler]

Ticket retargeted after milestone closed

[martinl]

All enhancement tickets should be assigned to 7.4 milestone.

[neteler]

Ticket retargeted after milestone closed

[martinl]

All enhancement tickets should be assigned to 7.6 milestone.

[martinl]

Ticket retargeted after milestone closed

[martinl]

Ticket retargeted after milestone closed