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

[datafactory] Regenerate datafactory to support new linked service types #3239

Merged
merged 10 commits into from
May 18, 2021

Conversation

fengzhou-msft
Copy link
Member

@fengzhou-msft fengzhou-msft commented Apr 8, 2021

Resolve #3223
Resolve #2872


This checklist is used to make sure that common guidelines for a pull request are followed.

General Guidelines

  • Have you run azdev style <YOUR_EXT> locally? (pip install azdev required)
  • Have you run python scripts/ci/test_index.py -q locally?

For new extensions:

About Extension Publish

There is a pipeline to automatically build, upload and publish extension wheels.
Once your PR is merged into master branch, a new PR will be created to update src/index.json automatically.
The precondition is to put your code inside this repo and upgrade the version in the PR but do not modify src/index.json.

@fengzhou-msft fengzhou-msft force-pushed the regenerate_datafactory branch from b331fd2 to 5a77375 Compare April 8, 2021 07:54
@yungezz yungezz requested review from qiaozha and qwordy April 9, 2021 02:15
@qiaozha
Copy link
Member

qiaozha commented Apr 10, 2021

@fengzhou-msft There're breaking changes between the previous generated code and now, such as az datafactory factory create => az datafactory create. I think you need to update the previous manual override part and the test part as well

@fengzhou-msft fengzhou-msft marked this pull request as ready for review April 26, 2021 09:54
factory_vsts_configuration=None,
factory_git_hub_configuration=None,
global_parameters=None):
def datafactory_create(client,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wondering why do we need to overide this? looks the same as the generated code ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In generated code, identity is empty: factory['identity'] = {}. Portal behavior is enabling system assigned identity by default, the change aligns the behavior with Portal.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe can try to set default value for identity ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and hide this parameter with directive

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parameter is already hidden. The default value is not a str, but an object, is it able to do so?

@qiaozha
Copy link
Member

qiaozha commented Apr 26, 2021

@changlong-liu Could you also take a look at this PR ? see if all these changes related with generated test or examples are expected ? Thanks

@changlong-liu
Copy link
Member

Should be fine. The test breaking change comes from some early requirements like Azure/autorest.az#600

Looks much manual efforts were spent on fixing test breaking changes. We may need to be more cautious on the test interface changes later.

@@ -101,6 +109,9 @@ def get_action(self, values, option_string): # pylint: disable=no-self-use
v = properties[k]
if kl == 'name':
d['name'] = v[0]
else:
raise CLIError('Unsupported Key {} is provided for parameter folder. All possible keys are: name'.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CLIError is deprecated

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generated code in extensions still uses CLIError to be compatible with older version of CLI.

@@ -10,7 +10,7 @@
from setuptools import setup, find_packages

# HISTORY.rst entry.
VERSION = '0.2.0'
VERSION = '0.1.0'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can version decrease?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caused by codegen change. The VERSION here will now be overwritten by from azext_datafactory.manual.version import VERSION and that is changed to 0.3.0 manually.

@fengzhou-msft fengzhou-msft merged commit 6bb827f into Azure:master May 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants