-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
Remove dependencies #13113
Merged
kceiw
merged 2 commits into
Azure:internal/Az.Tools.Predictor
from
kceiw:mahung/RemoveDependencies
Oct 8, 2020
Merged
Remove dependencies #13113
kceiw
merged 2 commits into
Azure:internal/Az.Tools.Predictor
from
kceiw:mahung/RemoveDependencies
Oct 8, 2020
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- That assembly and its dependencies causes a conflict if we load this module and Az.Accounts module. Though we can work around by sepcifying Az as a dependencies on this module, that'll cause long loading time. - The reason of loading that assembly is to get the setting about Azure PowerShell data collection. We can copy part of the code to load that file, so that we don't need that assembly as the dependency.
@albaloo @daxian-dbw @BethanyZhou Can you review it? |
/azp run azure-powershell - powershell-core |
Azure Pipelines successfully started running 1 pipeline(s). |
…nto mahung/RemoveDependencies
Hi @kceiw thanks for the contribution. Our team are just back from national holiday so sorry for late response. Anyway the changes LGTM :) |
@isra-fel Thanks. |
dingmeng-xue
added a commit
that referenced
this pull request
Nov 19, 2020
* psd1 & psm1 of Az.Installer with a demo cmdlet (#12698) * Internal/az.installer (#12817) * add util functions * add install-azmodule * add log to cmdlets * move verbose messages to debug * move under tools\ * polish warning message * move install-azmodule to exports * export install-azmodule * rename to az.tools.installer * Internal/az.installer (#12860) * add util functions * add install-azmodule * add log to cmdlets * move verbose messages to debug * move under tools\ * polish warning message * move install-azmodule to exports * export install-azmodule * rename to az.tools.installer * add uninstall-AzModule * Cmdlet Update-AzModule (#12832) * Update-AzModule * Address to PM requirements 1. Remove prefix Az. when input module name 2. update calculate from most recently installed respositories of different modules 3. reorder the sequence of module udpate and uninstall 4. other bug fix * Add telemetry * Support telemetry * Move Get-AzModuleUpdateList to internal (#12865) * add telemetry to Uninstall-AzModule cmdlet * bugfix: module name was not handled properly * Add environment variable to disable telemetry * parallel executing Install/Uninstall module * update code after review * improve performance * Fix Update-AzModule issues * bug fix * remove unused util * remove az.accounts when allow preview * Fix issue when no module to udpate * Add sign pipeline * skip confirmation when remove-job * remove debug info * fix parameter reference * Initial checking in AzPredictor (#12847) - It contains an implementation of ICommandPredictor that provides predictions to PSReadLine. - It also talks to the service endpoints to get the predictions. - It reads from the profile settings about the service endpoint. - There are some tests cases. * filter installedmodules before uninstall * bugfix for uninstall * Add synopsis and example to exported cmdlets * add default parameter set for uninstall-azmodule * add input validatin * remove unnecessary warning message * Build and package module under artifacts folder * Improve Az.Tool.Predictor (#12922) * Set up the dependencies on the PSReadline 2.1.0 * Initialize the env when the module is imported. - Add a script AzPredictor.ps1 to set the psreadline prediction view and source. - The script is run when the module is imported. * Fix a bug when loading settings from user profile. - The settings from user profile may not exist. In this case we should just ignore it and use the default one. * Fix issues in getting settings and send requests. - Fix the way to get the Azure PowerShell settings regarding collecting telemetry. - Avoid sending multiple requests while there is one running. * Use the NuGet pacakges from nuget.org. - Remove the local feed for those NuGet packages. * Rename namespace/assemblies/module name * Collect telemetry where we get the prediction. * Make some classes internal. * Add dependencies on Az module - This avoid the issue that our module loads Microsoft.Azure.PowerShell.Common dlls and Az tries to load it too. * Remove ps1 file that sets psreadline options. - We should instruct the users to set the options by themselves. * Remove psreadline module since it's not released. * Fix the merge issue * Add Az as external dependency * Add pipeline 'sign-tool-predictor' * Update sign-tool-predictor.yml for Azure Pipelines * Update sign-tool-predictor.yml for Azure Pipelines * net.webclient.downloadfile * refine command * clean build * Update sign-tool-predictor.yml for Azure Pipelines * Improve the build of Az.Tools.Predictor (#12974) * Change the build for Az.Tools.Predictor - Do a 'dotnet publish' instead of 'dotnet build' when the configuration is release. - When the configuration is release, replace the output folder with the ones from publish folder. The output folder is where the package retrieve the contents from. This will have the module and all the dependencies in the package. - The package target only runs when the configuration is release. - Add a target Test to run the unit tests. - Update the build command in azure pipeline to build with configuration release. * Specify the sub-folder artifact for each tool. - The build process involves deleting the artifact folders. And each tool shares the same artifact folder. This may be an issue if we build them in parralel because the build process of a tool will delete the folder while another tool is being built. - We'll use sub-folder for each tool in the artifact folder. Co-authored-by: Beisi Zhou <zhoubeisi@gmail.com> * Update sign-tool-predictor.yml for Azure Pipelines * Update sign-tool-predictor.yml for Azure Pipelines * Update sign-tool-predictor.yml for Azure Pipelines * Update sign-tool-predictor.yml for Azure Pipelines * Override powershellcorecommandPrefix * join_path * Update sign-tool-predictor.yml for Azure Pipelines * Update sign-tool-predictor.yml for Azure Pipelines * Update sign-tool-predictor.yml for Azure Pipelines * Update sign-tool-predictor.yml for Azure Pipelines * Install Az * Update sign-tool-predictor.yml for Azure Pipelines * Update sign-tool-predictor.yml for Azure Pipelines * Sign dll * Remove dll sign * Predict parameter values in the suggestion (#12984) * Get the parameter value from the history. * Add a mock ps console for testing purpose. - The mock ps console will echo back most of the commands. So that we don't need to really execute the Az command on Azure to test the prediction. * improve telemetry (#13025) * Correlate the telemetry event we send. - We have these telemetry entry points when we provide suggestion, when a suggestion is accepted, and when the user executes some command. We add a SuggestionId to the telemetry events so that we can correlate those three events. Doing that we know what suggestions are provided, accepted, and executed. * Collect and correlate our telemetry event. - With this change, we have these telemetry events; * RequestPrediction * GetSuggestion * AcceptSuggestion * CommandHistory - The events can be correlated by the session id and correlate id that are associate with the events. * Fix the request body. * Rename the prediction source. * Collect telemetry when an error occurs. * Remove some PII field * Collect user input in GetSuggestion event. * Use a common telemetry event prefix. * Remove the .gitignore that shouldn't be added. * Clean up the code. * return multiple suggestion (#13098) * Return multiple suggestions. - Add a setting to configure at most how many suggestions to return. - We'll go through the command/parameters one by one and to construct the suggestion. We may return multiple suggestions for the same command but with different parameter sets; * Use camel casing in the setting fields. - Looks like other json file in $HOME/.azure use camel case or PascalCase. None use snake case. * Fix a logic error. - We'll use the command list for suggestion while the prediction list isn't available. * Stop finding suggestion when the count is met. * Improve the MockPSConsole. * Cache the last two commands from the history. * Reduce the dependencies on Azure powershell common. (#13113) - That assembly and its dependencies causes a conflict if we load this module and Az.Accounts module. Though we can work around by sepcifying Az as a dependencies on this module, that'll cause long loading time. - The reason of loading that assembly is to get the setting about Azure PowerShell data collection. We can copy part of the code to load that file, so that we don't need that assembly as the dependency. * Add a missing file. (#13234) * Fix duplicate text (#13326) * Improve the debugging experience. - GetSuggestion gets cancelled during debugging because it times out. So we use an environment variable to control whether to allow cancellation from psreadline. - Use another macro to control if print message for telemetry. * Fix a bug that the command line is repeated in the suggestion. - When we parse the command line and there is an incomplete parameter list, we repeat the command line in the suggestion. - The cause is that we parse the command line and get the wrong parameter set from the user input * Update how we send request and collect telemetry (#13327) * Fix the query * Reduce the number of requests to the service. - We request the prediction for the command history. When the command history isn't changed, we don't need to request the prediction again. * Not to collect the parameter value in the telemetry. * Bug fixes and performance improvement (#13410) * Reduce extra cost to the perf issue. * Remove the aggregation on the parameter set. * Fix an issue * Fix some comments. * Improve the telemetry and http request (#13354) * Collect user id and mac address. - We hash the user account id and the mac address, and collect those in the telemetry. These are used to find how many users are using this mdoule. - We also send the hashed user account id in the http request header. This is used to enable throttling by user account. * Rename * Collect more data for diagnostic purpose. * Fix typo * Remove getting Az modules versions. * Update module infomation (#13328) * Set the description and dependencies for the module. * Update the project file and module manifest. * Update manifest and csproj * Fix typo * Move PSReadLine to ExternalModuleDependencies. - PSReadLine is in beta and there are issues in setting RequiredModules with a beta one. So move it as an ExternalModuleDependencies. * Update package script to support requiredmodule (#13424) * Add variety to Az.Predictor result list (#13420) * add variety to predictions list * add variety to predictions list * Update description for release * Add PublishLocation to build.proj of Az.Tools.Installer Co-authored-by: Jin Lei <54836179+msJinLei@users.noreply.github.com> Co-authored-by: Yabo Hu <yabhu@microsoft.com> Co-authored-by: msJinLei <leijin@microsoft.com> Co-authored-by: msJinLei <lei.jin@microsoft.com> Co-authored-by: kceiw <mahuang@microsoft.com> Co-authored-by: Beisi Zhou <zazbs@qq.com> Co-authored-by: Beisi Zhou <zhoubeisi@gmail.com> Co-authored-by: yemohyle <73853907+yemohyle@users.noreply.github.com>
dingmeng-xue
added a commit
that referenced
this pull request
Nov 20, 2020
* psd1 & psm1 of Az.Installer with a demo cmdlet (#12698) * Internal/az.installer (#12817) * add util functions * add install-azmodule * add log to cmdlets * move verbose messages to debug * move under tools\ * polish warning message * move install-azmodule to exports * export install-azmodule * rename to az.tools.installer * Internal/az.installer (#12860) * add util functions * add install-azmodule * add log to cmdlets * move verbose messages to debug * move under tools\ * polish warning message * move install-azmodule to exports * export install-azmodule * rename to az.tools.installer * add uninstall-AzModule * Cmdlet Update-AzModule (#12832) * Update-AzModule * Address to PM requirements 1. Remove prefix Az. when input module name 2. update calculate from most recently installed respositories of different modules 3. reorder the sequence of module udpate and uninstall 4. other bug fix * Add telemetry * Support telemetry * Move Get-AzModuleUpdateList to internal (#12865) * add telemetry to Uninstall-AzModule cmdlet * bugfix: module name was not handled properly * Add environment variable to disable telemetry * parallel executing Install/Uninstall module * update code after review * improve performance * Fix Update-AzModule issues * bug fix * remove unused util * remove az.accounts when allow preview * Fix issue when no module to udpate * Add sign pipeline * skip confirmation when remove-job * remove debug info * fix parameter reference * Initial checking in AzPredictor (#12847) - It contains an implementation of ICommandPredictor that provides predictions to PSReadLine. - It also talks to the service endpoints to get the predictions. - It reads from the profile settings about the service endpoint. - There are some tests cases. * filter installedmodules before uninstall * bugfix for uninstall * Add synopsis and example to exported cmdlets * add default parameter set for uninstall-azmodule * add input validatin * remove unnecessary warning message * Build and package module under artifacts folder * Improve Az.Tool.Predictor (#12922) * Set up the dependencies on the PSReadline 2.1.0 * Initialize the env when the module is imported. - Add a script AzPredictor.ps1 to set the psreadline prediction view and source. - The script is run when the module is imported. * Fix a bug when loading settings from user profile. - The settings from user profile may not exist. In this case we should just ignore it and use the default one. * Fix issues in getting settings and send requests. - Fix the way to get the Azure PowerShell settings regarding collecting telemetry. - Avoid sending multiple requests while there is one running. * Use the NuGet pacakges from nuget.org. - Remove the local feed for those NuGet packages. * Rename namespace/assemblies/module name * Collect telemetry where we get the prediction. * Make some classes internal. * Add dependencies on Az module - This avoid the issue that our module loads Microsoft.Azure.PowerShell.Common dlls and Az tries to load it too. * Remove ps1 file that sets psreadline options. - We should instruct the users to set the options by themselves. * Remove psreadline module since it's not released. * Fix the merge issue * Add Az as external dependency * Add pipeline 'sign-tool-predictor' * Update sign-tool-predictor.yml for Azure Pipelines * Update sign-tool-predictor.yml for Azure Pipelines * net.webclient.downloadfile * refine command * clean build * Update sign-tool-predictor.yml for Azure Pipelines * Improve the build of Az.Tools.Predictor (#12974) * Change the build for Az.Tools.Predictor - Do a 'dotnet publish' instead of 'dotnet build' when the configuration is release. - When the configuration is release, replace the output folder with the ones from publish folder. The output folder is where the package retrieve the contents from. This will have the module and all the dependencies in the package. - The package target only runs when the configuration is release. - Add a target Test to run the unit tests. - Update the build command in azure pipeline to build with configuration release. * Specify the sub-folder artifact for each tool. - The build process involves deleting the artifact folders. And each tool shares the same artifact folder. This may be an issue if we build them in parralel because the build process of a tool will delete the folder while another tool is being built. - We'll use sub-folder for each tool in the artifact folder. Co-authored-by: Beisi Zhou <zhoubeisi@gmail.com> * Update sign-tool-predictor.yml for Azure Pipelines * Update sign-tool-predictor.yml for Azure Pipelines * Update sign-tool-predictor.yml for Azure Pipelines * Update sign-tool-predictor.yml for Azure Pipelines * Override powershellcorecommandPrefix * join_path * Update sign-tool-predictor.yml for Azure Pipelines * Update sign-tool-predictor.yml for Azure Pipelines * Update sign-tool-predictor.yml for Azure Pipelines * Update sign-tool-predictor.yml for Azure Pipelines * Install Az * Update sign-tool-predictor.yml for Azure Pipelines * Update sign-tool-predictor.yml for Azure Pipelines * Sign dll * Remove dll sign * Predict parameter values in the suggestion (#12984) * Get the parameter value from the history. * Add a mock ps console for testing purpose. - The mock ps console will echo back most of the commands. So that we don't need to really execute the Az command on Azure to test the prediction. * improve telemetry (#13025) * Correlate the telemetry event we send. - We have these telemetry entry points when we provide suggestion, when a suggestion is accepted, and when the user executes some command. We add a SuggestionId to the telemetry events so that we can correlate those three events. Doing that we know what suggestions are provided, accepted, and executed. * Collect and correlate our telemetry event. - With this change, we have these telemetry events; * RequestPrediction * GetSuggestion * AcceptSuggestion * CommandHistory - The events can be correlated by the session id and correlate id that are associate with the events. * Fix the request body. * Rename the prediction source. * Collect telemetry when an error occurs. * Remove some PII field * Collect user input in GetSuggestion event. * Use a common telemetry event prefix. * Remove the .gitignore that shouldn't be added. * Clean up the code. * return multiple suggestion (#13098) * Return multiple suggestions. - Add a setting to configure at most how many suggestions to return. - We'll go through the command/parameters one by one and to construct the suggestion. We may return multiple suggestions for the same command but with different parameter sets; * Use camel casing in the setting fields. - Looks like other json file in $HOME/.azure use camel case or PascalCase. None use snake case. * Fix a logic error. - We'll use the command list for suggestion while the prediction list isn't available. * Stop finding suggestion when the count is met. * Improve the MockPSConsole. * Cache the last two commands from the history. * Reduce the dependencies on Azure powershell common. (#13113) - That assembly and its dependencies causes a conflict if we load this module and Az.Accounts module. Though we can work around by sepcifying Az as a dependencies on this module, that'll cause long loading time. - The reason of loading that assembly is to get the setting about Azure PowerShell data collection. We can copy part of the code to load that file, so that we don't need that assembly as the dependency. * Add a missing file. (#13234) * Fix duplicate text (#13326) * Improve the debugging experience. - GetSuggestion gets cancelled during debugging because it times out. So we use an environment variable to control whether to allow cancellation from psreadline. - Use another macro to control if print message for telemetry. * Fix a bug that the command line is repeated in the suggestion. - When we parse the command line and there is an incomplete parameter list, we repeat the command line in the suggestion. - The cause is that we parse the command line and get the wrong parameter set from the user input * Update how we send request and collect telemetry (#13327) * Fix the query * Reduce the number of requests to the service. - We request the prediction for the command history. When the command history isn't changed, we don't need to request the prediction again. * Not to collect the parameter value in the telemetry. * add variety to predictions list * add variety to predictions list * Bug fixes and performance improvement (#13410) * Reduce extra cost to the perf issue. * Remove the aggregation on the parameter set. * Fix an issue * Fix some comments. * Improve the telemetry and http request (#13354) * Collect user id and mac address. - We hash the user account id and the mac address, and collect those in the telemetry. These are used to find how many users are using this mdoule. - We also send the hashed user account id in the http request header. This is used to enable throttling by user account. * Rename * Collect more data for diagnostic purpose. * Fix typo * Remove getting Az modules versions. * Fixed issue of duplicate threshold consistency caused by two Query function calls as well as duplicate tolerance when list of novel commands is insufficiently small * Fixed issue of duplicate threshold consistency caused by two Query function calls as well as duplicate tolerance when list of novel commands is insufficiently small * remove commented lines, change Dictionary to IDictionary Co-authored-by: Jin Lei <54836179+msJinLei@users.noreply.github.com> Co-authored-by: Yabo Hu <yabhu@microsoft.com> Co-authored-by: dingmeng-xue <dixue@microsoft.com> Co-authored-by: msJinLei <leijin@microsoft.com> Co-authored-by: msJinLei <lei.jin@microsoft.com> Co-authored-by: kceiw <mahuang@microsoft.com> Co-authored-by: Beisi Zhou <zazbs@qq.com> Co-authored-by: Beisi Zhou <zhoubeisi@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This module used to depend on Az module which makes the loading time a little long. The reason why that's needed is to solve conflict when they both are loaded. They both depend on Microsoft.Azure.PowerShell.Common.dll and Microsoft.Azure.PowerShell.Authentication.Abstractions.dll. That cause conflicts in loading the assemblies. Once one module is loaded, the other one won't work. Though technically this module only augments Az module experience and it won't add anything if that doesn't exist, we can usually have Az module loaded when this module is used. The problem is loading Az module also takes a long time. The fix here is to remove the dependencies on Microsoft.Azure.PowerShell.Common.dll and Microsoft.Azure.PowerShell.Authentication.Abstractions.dll. We only need to find the configuration file about data collection. So I copied piece of the code to do that. This also bring down the our module size when we package it since we don't need to package the dependencies.
Checklist
CONTRIBUTING.md
ChangeLog.md
file(s) has been updated:ChangeLog.md
file can be found atsrc/{{SERVICE}}/{{SERVICE}}/ChangeLog.md
## Upcoming Release
header -- no new version header should be added