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

Design customization process from typespec generation #1950

Closed
MaryGao opened this issue Aug 1, 2023 · 17 comments
Closed

Design customization process from typespec generation #1950

MaryGao opened this issue Aug 1, 2023 · 17 comments
Assignees
Labels
Milestone

Comments

@MaryGao
Copy link
Member

MaryGao commented Aug 1, 2023

Background

We have customization requirement on generated code when we use the typespec as input. Currently we have openai and loadtesting SDK customized but with different approach. I think it's better to design clear and consistent expectations on:

  • How does the emitter handle customizations?
  • What is the customer experience during customization?
  • How does our process adopt into eng's scripts?

Option 1: Enable source-code-folder-path option

This is also the swagger way for customization and usually we could edit the config file inswagger/README.md under SDK folder(see example).

source-code-folder-path: ./src/generated

Different from swagger, in typespec all configs would save in spec repo which means both ci runs in spec pr and local gens in sdk repo would share the same config. To make it work we need to generate the top-level index.ts(if missing) to expose all api to public.

End user experience

  1. Add the option in tspconfig.yaml;
  2. Generate SDK by TypeSpec-Project-Process.ps1;
  3. Write their customization under src folder

Pros & Cons

  • Simple, effective and flexible
  • Sources would be located in different folders which is not recommeded by eng team(their comment here)

Option 2: Always generated in src/generated

Similar to Option 1, but here we would not differentiate the customization or not, the generated code would always in src/generated folder. Also we would detect if there is a top-level index.ts under src folder, if missing generate one. Customers can't point to any specific folder for generated code.

This option could work well if service team would have their customization codes in most cases, otherwise it seems redundant.

End user experience

  1. Generate SDK by TypeSpec-Project-Process.ps1;
  2. Write their customization under src folder

Pros & Cons

  • Simple and consistent cross any cases
  • The structure looks redundant in cases where no any customizations

Option 3: Use tool dev-tool customization apply

We have a tool(link) which could merge customization code and generated code. And the code structure would be like:

sdk/
├─ openai/
│  ├─ openai/
│  │  ├─ src/
│  │  │  ├─ ...
│  │  ├─ sources/
│  │  │  ├─ customizations/
│  │  │  ├─ generated/src...

To allow codegen to generate in sources/generated folder we need an option source-code-folder-path or allow-costomization.

End user experience

  1. Add the option source-code-folder-path: ./sources/generated
  2. Generate SDK by TypeSpec-Project-Process.ps1;
  3. Write their customization under sources folder;
  4. Run the command in SDK folder dev-tool customization apply -s sources/generated/src
  5. Re-generate SDK by TypeSpec-Project-Process.ps1;

To optimize above steps we could integrate customization tool as part of codegen, then the codegen's behavior would be like:

  1. Generate under ./sources/generated
  2. Run command customization apply to combine both parts
    2.1 If no customization, just copy to src
    2.2 If customization, combine them

Improved end user experience

  1. Generate SDK by TypeSpec-Project-Process.ps1;
  2. Write their customization under sources folder;
  3. Generate SDK by TypeSpec-Project-Process.ps1;

Pros & Cons

  • Heuristic way to detect if we have customizations
  • Clean combined code under src folder
  • But more complex compared with others
  • Feels non-intuitive to write code under ./sources/customizations

Option 4: In-place modify the generated code

This is Java's way to do customization(refer). They have @Generated annotation in generated code, so customers could modify them so their codegen could recognize it and keep these customizations.

End user experience

  1. Generate SDK by TypeSpec-Project-Process.ps1;
  2. Write their customization under src folder

Pros & Cons

  • Simple steps for customers and consistent generation
  • The codegen implementation would be quite complex to support different cases

Personally both Option 1 and Option 3 could work for me.

@xirzec @joheredi @qiaozha Could you take a look at above options and share your ideas?

/cc @lirenhe @raych1

@lirenhe lirenhe added the RLC label Aug 1, 2023
@xirzec
Copy link
Member

xirzec commented Aug 1, 2023

My personal preference is a hybrid of #2 and #3.

If the following two things are true:

  • The flag azureSdkForJs is true
  • The output directory contains the folder sources (fs.stat is used to check this at emit time)

Then we will not generate metadata files and will put only src output into sources/generated. For creating new packages that need customization, the generator can be run once, the sources folder can be manually created, and then the generator can be run a second time.

At this point dev-tool customize can be used to apply customizations to the generated output, it should work by default if run in a package folder that has sources/generated and sources/customizations

@joheredi @witemple-msft thoughts?

@xirzec
Copy link
Member

xirzec commented Aug 1, 2023

also /cc @deyaaeldeen since I think what I'm describing is basically a configuration-by-convention version of what he was trying to implement before his recent DTO

@deyaaeldeen
Copy link
Member

+1 on @xirzec proposal because it just works. We can iterate over it later if a need to arises.

@MaryGao MaryGao added this to the 2023-09 milestone Aug 2, 2023
@MaryGao MaryGao self-assigned this Aug 2, 2023
@MaryGao
Copy link
Member Author

MaryGao commented Aug 2, 2023

Thanks Jeff's input and I think we could start with this option and improve the process in future if necessary. And the experience of the hybrid option would be:

  • Run script
  • If customer has customization requirement for the first time,
    1. create sources folder,
    2. rerun script
  • Write customization code
  • Run devtool

@joheredi
Copy link
Member

joheredi commented Aug 2, 2023

I agree, 2 & 3 sound good and should work now. This is how I understand the process from a library developer perspective:

  • Run the Powershell script to generate
    • This generates under sources/generated
  • As part of the build the devtool customization apply is run
    • This even when there are no customizations. The tool would just copy the generated code
  • Iterate adding customizations under sources/custom if needed
  • Iterate re-generating with the Powershell script when needed.

@xirzec
Copy link
Member

xirzec commented Aug 2, 2023

The part that's up in the air to me is if we always generate into our repo like the customization tool is going to be used or if it's opt-in.

If it is opt-in, we need some mechanism to signal this. Since we got strong pushback on adding a new emitter flag (because the scripts and specs repo wants to only give us the package directory) would be to do something like checking for a subfolder being manually created.

That said, if we wanted to simplify by always having customization there and requiring that extra step when generating non-customized packages, I'm fine with that too. At least it makes it clear how to customize when you need it without needing to know the convention.

@joheredi
Copy link
Member

joheredi commented Aug 2, 2023

I'm fine with always generating as if we were having customization, and we can listen for partner feedback to decide if we want to implement some extra heuristics.

@qiaozha
Copy link
Member

qiaozha commented Aug 2, 2023

The Run script step means codegen will generate the code based on whether it can detect the sources folder of the output path, if there is, then we will generated into sources/generated folder, if there's no, then we will just generated into the normal src folder.

@qiaozha
Copy link
Member

qiaozha commented Aug 2, 2023

This is regardless if azure-sdk-for-js true or generate-metadata true. If we can make dev-tool public available, we can even generate the source code into sources/generated folder when working outside of the azure-sdk-for-js repo like codegen's own testing.

@MaryGao
Copy link
Member Author

MaryGao commented Aug 3, 2023

Personally I would prefer the opt-in option in emitter side so that the codegen could generate a runable SDK by themselve and don't need to depend on any external tools to finish this work. I may guess in most cases our SDKs may have no customizations, if any detecting a sources folder seems a reasonable heuristics.

Another point is that we may have scenarios to generate code outside of SDK repo e.g testing environment or external customers, running that tool could be challenging.

@joheredi How do you think?

@joheredi
Copy link
Member

joheredi commented Aug 3, 2023

Devtool already has some logic to detect project roots within libraries in azure-sdk-for-js. I guess if we can reuse that in the emitter to detect if running within the repo and generate under sources/generated (assume the azure-sdk-for-js flag to true).

This way we always generate under sources/generated for the repo and stand alone library when generating outside of the repo

@joheredi
Copy link
Member

joheredi commented Aug 3, 2023

FWIW I don't think we should make devtool public, it is tightly coupled by design to the mono repo so I'm not sure investing on opening it will give us good ROI

@MaryGao
Copy link
Member Author

MaryGao commented Aug 3, 2023

This way we always generate under sources/generated for the repo and stand alone library when generating outside of the repo

Just confirm my understanding is below:

This way we always generate under sources/generated for in azure-sdk-for-js repo and generate under src folder for stand alone library outside of the repo.

@MaryGao
Copy link
Member Author

MaryGao commented Aug 3, 2023

After offline discussion we have our final design. Please notice we would provide another command devtool customization init with customers for their first time and it will prepare the sources folder and copy code from src to generated.

1st-time of customer's experience

  • Run scripts to generate under src folder
  • If they have customization requirement (PS: if no, just skip this)
    • devtool customization init
    • write their customization code under sources/customizatiom
    • devtool customization apply
  • rushx build ...

2nd time to refresh customized SDK

  • Run scripts to refresh the SDK and code would be generated in sources/generated folder
  • devtool customization apply (PS: if no, just skip this)
  • rushx build ...

The ci steps in swagger pr

  • Run scripts to generate code
  • If there is sources folder, run command devtool customization apply
  • rushx build...

/cc @xirzec @lirenhe @deyaaeldeen

@lirenhe
Copy link
Member

lirenhe commented Aug 3, 2023

If service team already knows they need customization for the 1st time, can they run one script to ensure init code generated with all configuration and folder structure set correctly?

@MaryGao
Copy link
Member Author

MaryGao commented Aug 3, 2023

@lirenhe I don't think it is possible to run one script in our current design and I agree with @joheredi that we would listen for partner feedback to decide if we want to implement some extra heuristics/steps/flags. Our assumption is that if we could keep high quality codegen customization is not a requirement in most cases.

@MaryGao
Copy link
Member Author

MaryGao commented Aug 10, 2023

Close this issue considering the design is finalized.

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

No branches or pull requests

6 participants