[python] Add logic to clear output folder #8716
Conversation
|
All changed packages have been documented.
|
|
You can try these changes here
|
msyyc
requested review from
catalinaperalta,
iscai-msft,
kristapratico,
lmazuel,
mccoyp,
swathipil,
tadelesh and
timotheeguerin
as code owners
| # pylint: disable=too-many-branches | ||
| def serialize(self) -> None: | ||
| # remove existing folders when generate from tsp | ||
| if self.code_model.is_tsp and self.code_model.is_azure_flavor: |
There was a problem hiding this comment.
Shall we just filter the .py files with Code generated by Microsoft (R) Python Code Generator. to make the clean up much easier and more robust?
There was a problem hiding this comment.
We shall be very very careful about remove the .py file under generated SDK folder to avoid delete users' custom patched code. For example, if users copy the license title without any change then add their customized code, then your filter logic lose function.
Here are my thoughts:
(1) autorest.python has native flag --clear-output-folder of autorest core so don't need we do it in emitter
(2) for unbranded, we can't monitor users' code so it is better not add removal logic for now
(3) for azure, we could review their PR to make sure users don't add customized code randomly.
There was a problem hiding this comment.
Another option is we know the names of all of our generated files, so we can keep a list of the generated files and delete those on regeneration
There was a problem hiding this comment.
@iscai-msft Current logic is very similar as your solution that emitter will ONLY delete files under target output folder.
| await regenerate({ flavor: "azure", ...flags }); | ||
| await regenerate({ flavor: "unbranded", ...flags }); | ||
| } else { | ||
| await preprocess(flags); |
There was a problem hiding this comment.
I'm not comfortable with adding test code in the regen script. Is there any better way?
There was a problem hiding this comment.
We need logic to add the files before regeneration then test whether those files are kept or removed after regeneration. For now I can't find better way to do that.
There was a problem hiding this comment.
I'm not sure this is where we should put the preprocess call. This seems we only want to do it when we call regenerate with flags.
There was a problem hiding this comment.
@iscai-msft / @tadelesh I could also put preprocess before regenerate instead of inner it. Does it make sense? (e.g. I could convert preprocess to sync API then put it here)
There was a problem hiding this comment.
I meant more like line 473 if we always wanted it. Can we also add to_be_deleted.py to the gitignore so we don't accidentally commit it? Pipelines should still fail if it doesn't get deleted
msyyc
added
the
emitter:client:python
4 participants
fix Azure/azure-sdk-tools#11791
generation-subdirconfigured, added test case in this PRgeneration-subdirconfigured, since there is nogeneratedfolder, I have to add test case in autorest.python repo [python] Add logic to clear output folder Azure/autorest.python#3239