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

adjust blueprint generator scheduling #25445

Merged
merged 5 commits into from
Mar 9, 2024

Conversation

dwarakaprasad
Copy link
Contributor

@dwarakaprasad dwarakaprasad commented Mar 8, 2024

fixes #25444

A full blueprint (sbsBlueprint - false) will schedule immediately i.e. replacing the queue slot of the blueprinted generator.
A sbs blueprint (sbsBlueprint - true) will not affect the blueprinted generator's slot in queue but will take its own slot down below it.


Please make sure the below checklist is followed for Pull Requests.

When you are still working on the PR, consider converting it to Draft (below reviewers) and adding skip-ci label, you can still see CI build result at your branch.

@dwarakaprasad dwarakaprasad marked this pull request as ready for review March 8, 2024 11:23
@mshima
Copy link
Member

mshima commented Mar 8, 2024

This indeed is the correct behavior for full blueprints, but will break sbs blueprints.
Blueprint tasks will be queued before the blueprinted generator. sbs needs to be queued after.

The schedule parameter needs to be conditional, so it must be a function.
Should change here:
https://github.com/yeoman/environment/blob/1f2dd7bdcb9f9e58ed473c03a38afa0a58e63ecb/src/environment-base.ts#L428
to schedule: typeof schedule === ' function' ? schedule(generatorInstance) : schedule.

And add the conditional function before:

@dwarakaprasad
Copy link
Contributor Author

This indeed is the correct behavior for full blueprints, but will break sbs blueprints. Blueprint tasks will be queued before the blueprinted generator. sbs needs to be queued after.

The schedule parameter needs to be conditional, so it must be a function. Should change here: https://github.com/yeoman/environment/blob/1f2dd7bdcb9f9e58ed473c03a38afa0a58e63ecb/src/environment-base.ts#L428 to schedule: typeof schedule === ' function' ? schedule(generatorInstance) : schedule.

And add the conditional function before:

Thank you for your insight!

please correct me if i am wrong.
So to be on the same page,

blueprint = jhipster-<blueprint>:** namespace
blueprinted generator = jhipster:** namespace
A full blueprint is one where the sbsBlueprint is set to false
A full blueprint is one where the sbsBlueprint is set to true

As is today,

  1. A full blueprint's task should be queued first followed by the blueprinted generator's task i.e. should be scheduled immediately instead of going tro the environment:run loop.
    Currently composewithBlueprint method doesn't set the schedule value and so is defaulted to true and so all blueprints (full/sbs) are always being scheduled as of today. (not as expected)

  2. A sbs blueprint on the other hand should let its blueprinted generator's task to queue first and then get queued i.e. using the environment:run loop.
    same as above, sbs is always being scheduled as of today (same as expected)

Expectation is,

  1. A full blueprint should not be scheduled rather be queued immediately - so this mean schedule = false
  2. A sbs blueprint should be scheduled - so schedule = true

Instead of only having a binary possibility, you are proposing to have a function that will let generators make sophisticated decisions.

Since this proposal needs change to yeoman-environment, both releases will have to be timed.

@mshima
Copy link
Member

mshima commented Mar 8, 2024

Yes, that's it.

The only part that is not correct is:

  1. queued first followed by the blueprinted generator's task

The blueprinted generator's tasks are not queue unless the full blueprint inherits its tasks. The blueprinted generator is halt
Since the full blueprint completely overrides the blueprinted generator, it should replace its queue slot by queueing immediately too.

Instead of only having a binary possibility, you are proposing to have a function that will let generators make sophisticated decisions.

Should be:
schedule: generator => generator.sbsBlueprint

Since this proposal needs change to yeoman-environment, both releases will have to be timed.

No problem with that.
Are you willing to create the PR?

@dwarakaprasad
Copy link
Contributor Author

it should replace its queue slot by queueing immediately too.

Yes, it makes sense.

Are you willing to create the PR?

Yes...

@dwarakaprasad
Copy link
Contributor Author

@mshima changes made

  1. @yeoman/type changes
  2. environment change
  3. jhipster change

@dwarakaprasad dwarakaprasad changed the title prioritize language blueprint over other blueprints change blueprint generator scheduling Mar 9, 2024
mshima pushed a commit to yeoman/yeoman-api that referenced this pull request Mar 9, 2024
* add call back function support to deceide generator scheduling

requiredby jhipster/generator-jhipster#25445

* incorporate review suggestions
mshima pushed a commit to yeoman/yeoman-api that referenced this pull request Mar 9, 2024
…uling (#6)

* add call back function support to deceide generator scheduling

requiredby jhipster/generator-jhipster#25445

* incorporate review suggestions
@mshima mshima closed this Mar 9, 2024
@mshima mshima reopened this Mar 9, 2024
@mshima mshima changed the title change blueprint generator scheduling adjust blueprint generator scheduling Mar 9, 2024
@mshima mshima merged commit 89e9e7b into jhipster:main Mar 9, 2024
65 of 79 checks passed
@deepu105 deepu105 added this to the 8.2.0 milestone Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

angular & language generators combined in a blueprint throws error during unit testing
3 participants