Expected structure of a 'tag' is unclear.

[danielballan]

These function signatures make it very clear what the allowed parameters are, just by looking at them:

https://github.com/NSLS-II/olog/blob/a04f1055df76c3ce62f84e31eea37180bab33b67/olog/httpx_client.py#L95-L97

https://github.com/NSLS-II/olog/blob/a04f1055df76c3ce62f84e31eea37180bab33b67/olog/httpx_client.py#L148-L151

This makes them easy to guess how to use. This function signature does not, because the parameter tag has an internal structure, and the user has to "just know" what that structure is supposed to be.

https://github.com/NSLS-II/olog/blob/a04f1055df76c3ce62f84e31eea37180bab33b67/olog/httpx_client.py#L182-L195

Per the olog-es REST API documentation it seems that the stricture of tag must include a name key. To my reading, it's not clear from the documentation whether a state key is also required or optional, and what values it might be allowed to have other than 'Active'. Perhaps the signature could be simplified to:

async def aput_tag(self, name):

or

async def aput_tag(self, name, state="Active"):

depending on the currently-unknown-to-me details of what state means here. I see a tag listed with just a name and no other key in this example.

[ke-zhang-rd]

it seems that the stricture of tag must include a name key.

From my observation, that's correct. Any also it ignore other key(I randomly tried some, llike foo and bar) won't be put into database except name and state.

I think this is ok if service doesn't expect any other special key, @shroffk? Also, if we use name as only argument, we could remove

if tag != res.json():

[shroffk]

The service only expects the tag.name

The tag.state is part of maintaining the life cycle of a tag... according to a lot of DOE requirements nothing should be removed. The delete thus changes the state from Active to Inactive thus old log entries can be kept consistent while still allowing filterning and hiding of tags, logbooks, properties for future log entries.

[shroffk]

async def aput_tag(self, name): or

async def aput_tag(self, name, state="Active"):

both are fine...in my opinion. I will add the explanation for the state in the docs

[danielballan]

Thanks @shroffk. Given that, I move to change our signatures to

async def aput_tag(self, name):

and

async def aput_tags(self, names):

and also add

async def delete_tag(self, name):

to exercise the DELETE method, which will cause the server to change the state from 'Active' to 'Inactive'.

[shroffk]

async def delete_tag(self, name): to exercise the DELETE method, which will cause the server to change the state from 'Active' to 'Inactive'.

we can use this to test the server but I feel like maybe we should only expose this part of the API when we have a good argument for it.

[ke-zhang-rd]

closed by #25