Skip to content

Conversation

@Ahmad-Wahid
Copy link
Contributor

Fixes:
image

@Ahmad-Wahid Ahmad-Wahid self-assigned this Apr 30, 2025
@Ahmad-Wahid Ahmad-Wahid added the bug Something isn't working label Apr 30, 2025
@Ahmad-Wahid Ahmad-Wahid requested a review from nhoening April 30, 2025 12:19
Copy link
Contributor

@nhoening nhoening left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@Flix6x
Copy link
Contributor

Flix6x commented Apr 30, 2025

Why was this needed, actually? I would have expected only a single source when filtering by name, model, version and type. Maybe this points to a missing unique constraint on the datasource table?

For your consideration: is silently selecting the first among multiple sources worth logging a warning? Or is this a regular expected case?

@Ahmad-Wahid
Copy link
Contributor Author

Ahmad-Wahid commented Apr 30, 2025

Why was this needed, actually? I would have expected only a single source when filtering by name, model, version and type. Maybe this points to a missing unique constraint on the datasource table?

For your consideration: is silently selecting the first among multiple sources worth logging a warning? Or is this a regular expected case?

We have got three data sources(duplicate) and i think all of them have data. I'm not able to query data even after applying a limit.

image

@Flix6x
Copy link
Contributor

Flix6x commented May 1, 2025

My recommendations:

  • check if creating another duplicate data source leads to a db error
    • if it doesn't raise a db error, we could discuss a db migration where we add a suitable constraint and we could add a CLI command to merge duplicate sources
    • if it does, then I'm puzzled at how that db ended up in this state, but we may still write a CLI option to merge sources.
  • in case this function runs into multiple sources, we could raise and refer to the new CLI command.
  • the smart-buildings plugin is using the source name to uniquely identify a data source. Perhaps it should be using an ID instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants