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

Add @Async annotation for the commands/arguments #401

Open
vortexthedev opened this issue Sep 11, 2023 · 8 comments
Open

Add @Async annotation for the commands/arguments #401

vortexthedev opened this issue Sep 11, 2023 · 8 comments

Comments

@vortexthedev
Copy link

No description provided.

@SumanohariR
Copy link

Hello, I hope you're all doing well!
Could you please confirm whether this issue has already been assigned to someone? If it hasn't it would be a pleasure to contribute to your project!

@chickeneer
Copy link
Collaborator

I would confirm that no one is working on this "issue". Truthfully, I have not put any thought into what this looks like to implement. Or anything else about it.

@SumanohariR
Copy link

It would be helpful if you could provide a description of the issue! @vortexthedev @chickeneer

@vortexthedev
Copy link
Author

It would be helpful if you could provide a description of the issue! @vortexthedev @chickeneer

The title of issue is saying everything

@vortexthedev
Copy link
Author

Add @async annotation, so we can get rid of non-async database queries for example in the registered contexts (params).

@vortexthedev
Copy link
Author

vortexthedev commented Oct 5, 2023

I would confirm that no one is working on this "issue". Truthfully, I have not put any thought into what this looks like to implement. Or anything else about it.

Could you implement next methods? (If any of registered contexts are async, then the whole command execution will be off main thread by using runTaskAsync?)

CommandManager#registerAsyncContext
CommandManager#registerAsyncOptionalContext
CommandManager#registerAsyncIssuerAwareContext

Add @async annotation so we can select any commands besides the params to be async.

@Joo200
Copy link
Contributor

Joo200 commented Oct 5, 2023

No, there are some open things:

  • How should acf handle async resolvers. Should they be registered and instead of returning the object directly a CompletionStage? Or should the context resolver simply be called async?
  • Should the execution of the command be async (off main thread) or rescheduled into the main thread?
  • Since it's possible to access values from other arguments with the provided context in the context resolver there should be more information on how to implement this feature for async contexts.

Feel free to make a PoC by forking acf yourself and try to implement such a feature. If anyone could implement that in a few minutes we would already added it as a feature to acf. But such a complex idea should be implemented correctly and the required time for this is huge.

@vortexthedev
Copy link
Author

vortexthedev commented Oct 13, 2023

If there is any parameters that are required to be run async, or the command/subcommand has @async annotation, then the whole command process (Command#execute) will be ran through Scheduler#runTaskAsync().
By the command process I mean retrieving the parameters values through registered contexts and running the command method.

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

4 participants