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

Integrate bal persist generate to the bal build #5784

Closed
daneshk opened this issue Nov 16, 2023 · 13 comments
Closed

Integrate bal persist generate to the bal build #5784

daneshk opened this issue Nov 16, 2023 · 13 comments

Comments

@daneshk
Copy link
Member

daneshk commented Nov 16, 2023

Description:
We need to integrate the bal persist generate to the bal build command as now bal build supports integrating external tools to the bal build so that it will generate the necessary source code in the pre-compile stage.

Lang Changes are available at: /~https://github.com/ballerina-platform/ballerina-lang/tree/new-tool-build-integration

You can use the following samples to get started.
/~https://github.com/Dilhasha/sample-build-tool-runner
/~https://github.com/ShammiL/sample-build-tool

@daneshk
Copy link
Member Author

daneshk commented Nov 16, 2023

According to the new tool integration support, there are a few changes we need to make in the persist configuration in the Ballerina.toml file.

If the datastore is MySQL, the package name is hospitalsvc and the module name is db

  • With the current support,
[persist]
datastore = "mysql"
module = "hospitalsvc.db"
  • With the proposed design,
[tool.persist]
id = "generate-db-client"
filePath = "persist/model.bal" // required field
targetModule = "db"
options.datastore = "mysql"

In order to maintain backward compatibility, we do need to support both config formats in the bal persist generate command.
The following scenarios need to support,

  • Create a new project in update 09 and generate the client using the bal persist generate command.
    • bal persist init should generate the new config and bal persist generate should support both configs
  • Create a new project in update 09 and generate the client using bal build.
  • Run the existing project in update 09
    • The bal build command never tries to generate the client as Ballerina.toml configs are different.
  • Change the model definition and regenerate the client using the bal persist generate command in update 09
    • bal persist generate should support both configs
  • For the existing projects having both configs in the Ballerina.toml file and having different values.
    • Give priority to the new config format as we need consistency in bal build and bal persist generate
  • For the existing projects, try to do the bal persist init command.
    • Currently, we are not allowed to do bal persist init when there is a [persist] config in the Ballerina.toml file. User need to remove the config and execute. With this, we need to check both [persist] and [tool.persist] config and generate [tool.persist] config, if there is no config in the toml file.

@daneshk
Copy link
Member Author

daneshk commented Nov 26, 2023

Based on the discussion, we need to focus on two use cases when designing the bal build integration,

  • Developers who don't want to commit generated code with their application source code. They expect the application build will autogenerate every time at the pre-compile phase.
  • Developers who want to commit generated code as a part of their application source code.

We need to identify the use case when initializing the bal persist and record it in the Ballerina.toml file. So we can decide based on the toml configurations.

bal persist init --datastore mysql --module db --integrate-build false

This will generate Ballerina.toml config as follows,

[persist]
datastore = "mysql"
module = "hospitalsvc.db"

This will generate the client in a submodule called db which is not in the generated directory and can commit with the rest of the project code.

Additional param --integrate-build is optional, if it is not provided, it defaults integrates to the bal build and generated code is in the generated directory which is not committed. The Ballerina.toml configs will be as follows,

[tool.persist]
id = "generate-db-client"
filePath = "persist/model.bal" // required field
targetModule = "db"
options.datastore = "mysql"

@daneshk
Copy link
Member Author

daneshk commented Nov 26, 2023

@sameerajayasoma Please review and share your thoughts on the latest design

@daneshk daneshk added Type/Proposal Status/Draft In circulation by the author for initial review and consensus-building labels Nov 26, 2023
@daneshk
Copy link
Member Author

daneshk commented Nov 27, 2023

Possible Scenarios and Proposed Approach

Possible Scenarios Proposed Approach
Create a new project and execute the bal persist init command * The bal persist generate command failed with an error.
* The client is generated in bal build command.
* The client source code is generated inside generated directory.
Create a new project and execute the bal persist init --integrate-build false command * The client is generated in bal persist generate command.
* The client is not generated in bal build command.
* The client source code is generated inside the submodule specified in the Ballerina.toml file.
Regenerate the client for the existing project * The client is generated in bal persist generate command.
* The client is not generated in bal build command.
* The client source code is generated inside the submodule specified in the Ballerina.toml file.
* If the generated clients are in the generated directory, the user needs to manually delete them from the project. This happens, if the user commit the generated client, no need to change any user code.
Regenerate the client for the existing project, but needs integrate with the bal build command * The user needs to delete persist configuration from the Ballerina,toml file first and then execute the bal persist init command.
* The client is not generated in bal persist generate command.
* The client is generated in bal build command.
* The client source code is generated inside the generated directory and module specified in the Ballerina.toml file.
Reinitialize bal persist for the existing project * The user needs to delete persist configuration from the Ballerina,toml file first and then execute the bal persist init command. This apply for both persist and tool.persist configuration.
* If there is any for the persist configuration, failed giving an error.
Manually add the persist configuration to the Ballerina.toml file * The bal persist generate and bal build command will check whether both configures are available in the Ballerina.toml file and failed giving an error

@sameerajayasoma
Copy link
Contributor

As you've described, there are two distinct use cases.

  1. Generate the client code against the model and commit the generated code as part of the source code.
  2. Integrate the client code generation with the package build to avoid committing generated code.

Both scenarios have pros and cons and depend on a particular project's requirements. We need to support both.

This is my proposal to improve bal openapi command to support above scenarios.

bal openapi add  - This command adds an entry to "Ballerina.toml" to integrate the code generation with the package build process.
bal openapi generate - This command performs a one-time generation of the client.

I think we can model the bal persist CLI design based on the above design. But in the bal openapi case, the OAS is already there. The command just generates the code. In bal persist, you don't have a model at the beginning. We need to help the developer prepare the package by creating the necessary files and directories to work on the model.

bal persist add command

This command adds a code generation task to the "Ballerina.toml". This task will be executed by the bal build and the generated code will be part of the generated directory.

bal persist add --datastore mysql --module db

This element will be added to the Ballerina.toml file.

[tool.persist]
id = "generate-db-client"
targetModule = "db"
options.datastore = "mysql"

bal persist generate command

Trigger a one-time generation. This command does not need to be package-aware for most of the cases. Generates the code in the given output location.

bal persist generate --datastore mysql 

The generated code will be placed in the current directory. IMO, we don't need to store anything in the "Ballerina.toml". You can use the same command whenever you need to re-generate the code.

@sameerajayasoma
Copy link
Contributor

FYI, my proposal introduces breaking changes. If the design is right, I don't think we should delay introducing these changes. The current design has several loopholes.

Btw, the above proposal does not address the creation of the persist directory.

  • Option 1: We can ask the developer to create the persist directory and a model.bal file.
  • Option 2: Repurpose bal persist init to create this directory and the model file.

@daneshk
Copy link
Member Author

daneshk commented Dec 7, 2023

The bal persist generate command should also be package-aware as it needs to read the model definition inside the package. So the following are the behaviors of each command after the proposed changes

bal persist generate command

  • Trigger a one-time generation
  • It is package-aware. It should execute within the package and a model definition file should be inside the persist directory.
  • It does not read any configuration in the Ballerina.toml and we need to pass the parameters along with the command like,
bal persist generate --datastore mysql --module db

Here, the datastore parameter is mandatory and the module is optional. If the module is not specified, files are generated inside the default package.

  • The generated files can be committed along with the user code and they are not generated inside the generated directory.
  • it adds necessary native dependencies in the Ballerina.toml file.
    e.g:
[[platform.java17.dependency]]
groupId = "io.ballerina.stdlib"
artifactId = "persist.sql-native"
version = "1.2.2"

bal persist init command

  • It is package-aware.
  • It only creates the persist directory and a model.bal file.
  • It doesn't accept any parameters. The command should be like,
bal persist init
  • The user can even omit this command and create the persist directory and a model.bal file manually inside the package.

bal persist add command

  • It is package-aware.
  • Integrates source generation to the bal build command.
  • It creates the persist directory and a model.bal file, if doesn't exist.
  • The command should be like,
bal persist add --datastore mysql --module db

Here, both the datastore parameter and the module are optional.

  • Files are generated inside the default package if the module is not specified.
  • Consider inmemory datastore if the datastore is not specified
  • It saves the above parameters inside the Ballerina.toml file like below and uses them in the bal build command.
[tool.persist]
id = "generate-db-client"
targetModule = "db"
options.datastore = "mysql"
  • it adds necessary native dependencies in the Ballerina.toml file.
    e.g:
[[platform.java17.dependency]]
groupId = "io.ballerina.stdlib"
artifactId = "persist.sql-native"
version = "1.2.2"
  • The command can only execute once, if the user needs to re-execute, he/she needs to remove the configs added in the Ballerina.toml file.
  • Not recommended to change the configs, as this will affect the generated code.
  • Once we execute the bal build command, the generated code will be part of the generated directory.

bal persist migrate command

  • It is package-aware.
  • The command should be like,
bal persist migrate add_employee --datastore mysql

Here both label and datastore parameters are mandatory for each execution.

  • Should have bal persist initiated in the project and have a model definition file.
  • Can execute the command multiple times. It will generate the migration scripts based on the changes in the model definition file.
  • Once the migration is generated, you cannot change the data store type. If you want to change the data store type, you need to remove the migration directory.

@daneshk
Copy link
Member Author

daneshk commented Dec 8, 2023

For migrating existing projects

bal persist generate command

Things to consider

  • The configuration in the Ballerina.toml file is redundant and we need to remove the configs
  • The generate command requires datasource and module as parameters now.
  • The user will look for generated source files in the generated directory. but it generates in the user code.

Migration Plan

  • When bal persist generate executes without params, ask for the datasource parameter. This may confuse the user, but as we mandate the datastore now, we can't do custom messages here.
  • When bal persist generate --datastore <> executes, check the Ballerina.toml file for the old [persist] configuration and ask the user to remove the old configs and generated source files and re-execute the command.

Sample message:

The behavior of the `bal persist generate` command has been updated starting from Ballerina update 09. 
You now have the following options for code generation:

- `bal persist add --datastore <datastore> --module <module>`: This command adds an entry to "Ballerina.toml" to integrate code generation with the package build process.
- `bal persist generate --datastore <datastore> --module <module>`: This command performs a one-time generation of the client.

If you choose to proceed with the `bal persist generate` command, please follow these steps:

1. Remove the [persist] configuration in the "Ballerina.toml" file.
2. Delete any previously generated source files, if they exist.
3. Re-execute the `bal persist generate --datastore <datastore> --module <module>` command.

If you have any questions or need further assistance, refer to the updated documentation.

For the new projects which don't have the old [persist] configuration, we simply say,

Persist client and entity types generated successfully in the <module name>/default module.

bal persist init command

Things to consider

  • The init command doesn't require any parameters now.

Migration Plan

  • Instead of removing the datastore and module parameters, we can make them optional. Otherwise the existing users with get an error message like ballerina: unknown options: '--module', 'db'. If they provide values for datastore and module parameters, We can give the message like below,

Sample message:

The behavior of the `bal persist init` command has been updated starting from Ballerina update 09. 
Now, you no longer need to provide any parameters. The command will exclusively create the `persist` directory in the project's root directory. This directory contains the data model definition file (`model.bal`) for the project.

If you have any questions or need further assistance, refer to the updated documentation.

For the new projects and the user doesn't specify any parameters, we can simply create the directory and display the message like,

Initialized the package for persistence.

Next steps:
1. Define your data model in "persist/model.bal".
2. Execute `bal persist add --datastore <datastore> --module <module>` to add an entry to "Ballerina.toml" for integrating code generation with the package build process.
OR
Execute `bal persist generate --datastore <datastore> --module <module>` for a one-time generation of the client.

@daneshk
Copy link
Member Author

daneshk commented Dec 8, 2023

@sameerajayasoma Please share your thoughts on migration plan

@sameerajayasoma
Copy link
Contributor

LGTM

@sameerajayasoma
Copy link
Contributor

For any project that is migrating to Update 9, everything will work as usual. Only when you try to re-generate code using bal persist generate, we give the above message.

@daneshk
Copy link
Member Author

daneshk commented Dec 14, 2023

For any project that is migrating to Update 9, everything will work as usual. Only when you try to re-generate code using bal persist generate, we give the above message.

Yes, correct. The above message will only print until he/she cleans up the old configs in Ballerina.

@daneshk
Copy link
Member Author

daneshk commented May 9, 2024

This is done. Hence closing the issue

@daneshk daneshk closed this as completed May 9, 2024
@github-project-automation github-project-automation bot moved this from BackLog to Done in Ballerina Team Main Board May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

2 participants