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.

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

Merge k4DataSvc and PodioDataSvc #104

Open
tmadlener opened this issue May 19, 2023 · 7 comments · May be fixed by #163
Open

Merge k4DataSvc and PodioDataSvc #104

tmadlener opened this issue May 19, 2023 · 7 comments · May be fixed by #163

Comments

@tmadlener
Copy link
Contributor

As discussed in #100 k4DataSvc is effectively just a very thin wrapper around PodioDataSvc and can be removed. This issue is mainly for keeping track of the removal and for related discussions.

@tmadlener tmadlener added enhancement New feature or request and removed enhancement New feature or request labels May 19, 2023
@jmcarcell
Copy link
Member

k4DataSvc is used in many repos both in key4hep and other organizations so it's a fair amount of work for not much gain:

@tmadlener
Copy link
Contributor Author

Dropping PodioDataSvc in favor of k4DataSvc is fine for me. The main goal is to remove the sort of artificial split.

@tmadlener tmadlener changed the title Drop k4DataSvc in favor of PodioDataSvc Merge k4DataSvc and PodioDataSvc Aug 15, 2023
@tmadlener
Copy link
Contributor Author

There is another DataSvc FCCDataSvc that effectively does the same as k4DataSvc.

@kjvbrt @BrieucF is that still in use to the best of your knowledge?

@tmadlener
Copy link
Contributor Author

tmadlener commented Nov 17, 2023

There might be a technical reason for this split(?) At least simply merging PodioDataSvc into k4DataSvc and moving that to the core k4FWCore library, results in

[warning] library [k4FWCorePlugins] exposes factory [k4DataSvc] which is declared in [libk4FWCore.so] !!

and then subsequently runtime crashes with:

[k4run] : Configurable k4DataSvc not found in module k4FWCore.k4FWCorePluginsConf
Traceback (most recent call last):
  File "/home/tmadlener/work/key4hep/k4FWCore/k4FWCore/scripts/k4run", line 129, in <module>
    load_file(file)
  File "/home/tmadlener/work/key4hep/k4FWCore/k4FWCore/scripts/k4run", line 26, in load_file
    exec(file.read(), {})
  File "<string>", line 22, in <module>
TypeError: 'NoneType' object is not callable

EDIT: That is solvable with putting the DECLARE_COMPONENT into the right place

@tmadlener tmadlener linked a pull request Nov 17, 2023 that will close this issue
4 tasks
@BrieucF
Copy link
Contributor

BrieucF commented Nov 17, 2023

There is another DataSvc FCCDataSvc that effectively does the same as k4DataSvc.

@kjvbrt @BrieucF is that still in use to the best of your knowledge?

Hi Thomas, yes FCCDataSvc is used in some places outside of the Key4hep/HEP-FCC organizations but I can do the modification where I know it is used and advertise the change to the relevant mailing lists to warn people if it is decided to remove FCCDataSvc.

@kjvbrt
Copy link
Contributor

kjvbrt commented Nov 17, 2023

We removed it in some places, but not all, yet.

@tmadlener
Copy link
Contributor Author

OK. Thanks for checking. We could put in some deprecation warning log message before completely removing it. That would at least make people aware if they are checking their logs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants