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

entry point names do not follow convention #13

Closed
ltalirz opened this issue Aug 30, 2021 · 9 comments
Closed

entry point names do not follow convention #13

ltalirz opened this issue Aug 30, 2021 · 9 comments

Comments

@ltalirz
Copy link

ltalirz commented Aug 30, 2021

hey, just letting you know that the entry point naming does not follow AiiDA conventions:

"aiida.schedulers": [
"fireworks = aiida_fireworks_scheduler.fwscheduler:FwScheduler"
],
"aiida.cmdline.data": [
"fireworks-scheduler = aiida_fireworks_scheduler.cmdline:fw_cli"
],

The canonical entry point prefix would be fireworks_scheduler

@ltalirz
Copy link
Author

ltalirz commented Aug 30, 2021

P.S. Specifying an compatible version range of aiida-core may also be useful.

@zhubonan
Copy link
Owner

@ltalirz to double check - you meant the entry point for aiida.cmdline.data?

@ltalirz
Copy link
Author

ltalirz commented Aug 30, 2021

both of them - please use fireworks_scheduler (or fireworks_scheduler.abc)

@zhubonan
Copy link
Owner

But the AiiDA built in names for schedulers don't have the suffix '_scheduler' (e.g. she, PBS)?

Also, changing this would break backward compatibility - name of the scheduler is stored in the Computer node which are immutable.

@ltalirz
Copy link
Author

ltalirz commented Aug 30, 2021

Thanks, aiida-core was indeed an exception to the convention so far and we've been thinking about whether to enforce it or not.
Will let you know what comes out of the discussion later today.

P.S. Breaking backwards compatibility would indeed not be ideal. However, one could at least add a new entry point that follows the convention and points to the same thing.

@ltalirz
Copy link
Author

ltalirz commented Aug 31, 2021

the decision was made to help clarify the namespacing by adding a core. prefix for entry point names from aiida-core aiidateam/aiida-core#5073 (comment)

the actual change will be made at some point in the future, but with 2.0 we will add deprecation warnings for entry point without the prefix.
of course, in aiida-core we have the ability to migrate entry point names; migration of nodes in plugins is still an unsolved problem.

for your plugin I would suggest to keep the fireworks entry point name for backwards compatibility but eventually (with the 2.0 update) introduce a properly namespaced entry point name that points to the same thing.

@zhubonan
Copy link
Owner

OK, thanks for letting me know! For the cmdline entry point I will change the name as it does not have any problem.

This to confirm - following the convention:

"aiida.schedulers": [
"fireworks = aiida_fireworks_scheduler.fwscheduler:FwScheduler"
],
"aiida.cmdline.data": [
"fireworks-scheduler = aiida_fireworks_scheduler.cmdline:fw_cli"
],

should be:

 "aiida.schedulers": [ 
     "fireworks_scheduler.fireworks = aiida_fireworks_scheduler.fwscheduler:FwScheduler" 
 ], 
 "aiida.cmdline.data": [ 
     "fireworks_scheduler.cmd = aiida_fireworks_scheduler.cmdline:fw_cli"  
 ], 

?

I will change the entry point name for aiida.cmdline.data, as there is no issue with backward compatibility.

@ltalirz
Copy link
Author

ltalirz commented Aug 31, 2021

Yes, for example.

We still need to decide whether we also allow fireworks_scheduler as an entry point for cases like these where you are only adding one entry point per group. Anyhow, using fireworks_scheduler.abc as you do here is safer in case you ever want your plugin to add more than one entry point to a given group

As mentioned, you can keep the old entry point name for the scheduler around for backwards compatibility if you like

@zhubonan
Copy link
Owner

zhubonan commented Nov 25, 2021

@ltalirz Thanks, I have decided to go with:

        "aiida.schedulers": [
            "fireworks = aiida_fireworks_scheduler.fwscheduler:FwScheduler",
            "fireworks_scheduler.default = aiida_fireworks_scheduler.fwscheduler:FwScheduler",
            "fireworks_scheduler.keepenv = aiida_fireworks_scheduler.fwscheduler:FwSchedulerKeepEnv"
        ],

Having the prefix is very useful, it turns out there needs to be two ways of launching aiida jobs when task farming, depending on what type of real scheduler is used.

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

No branches or pull requests

2 participants