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

Add environment list retrieval and refactor BAP endpoint logic #1060

Merged
merged 5 commits into from
Nov 22, 2024

Conversation

amitjoshi438
Copy link
Contributor

@amitjoshi438 amitjoshi438 commented Nov 20, 2024

This pull request includes several changes to the BAPService class and related utilities to enhance the functionality and improve code readability. The most important changes include renaming methods for clarity, adding new telemetry constants, and introducing a new utility function to fetch environment lists.

Key changes:

Enhancements to BAPService:

  • Renamed the getBAPEndpoint method to getBAPCopilotCrossGeoFlagEndpoint for better clarity and updated its implementation to use the new getBAPEndpoint utility function.
  • Updated the getCrossGeoCopilotDataMovementEnabledFlag method to call the renamed getBAPCopilotCrossGeoFlagEndpoint method.

New utility function:

  • Added a new utility function getEnvList in Utils.ts to fetch the environment list for an organization, including necessary telemetry and error handling.

Telemetry constants:

  • Introduced new telemetry constants VSCODE_EXTENSION_GET_ENV_LIST_SUCCESS and VSCODE_EXTENSION_GET_ENV_LIST_FAILED to track the success and failure of fetching environment lists.

Additional imports:

  • Added necessary imports in Utils.ts to support the new utility function and telemetry constants.

Constants:

  • Added BAP_ENVIRONMENT_LIST_URL constant to Constants.ts for the new environment list endpoint.

@amitjoshi438 amitjoshi438 requested review from a team as code owners November 20, 2024 10:29
Copy link
Contributor

@priyanshu92 priyanshu92 left a comment

Choose a reason for hiding this comment

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

Few small comments otherwise LGTM

src/common/services/Constants.ts Outdated Show resolved Hide resolved
src/common/utilities/Utils.ts Outdated Show resolved Hide resolved
src/common/utilities/Utils.ts Show resolved Hide resolved
@amitjoshi438 amitjoshi438 merged commit c7d4ff9 into main Nov 22, 2024
6 checks passed
@amitjoshi438 amitjoshi438 deleted the users/amitjoshi/getEnvList branch November 22, 2024 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants