Skip to content

Conversation

@shenmuxiaosen
Copy link
Contributor

  • Fix bug when appending api-version to request url. The existing solution doesn't work with pagination.
  • For globalization, we should support showing languages besides English as our backend service support unicode.

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

  • The PR has modified HISTORY.rst describing any customer-facing, functional changes. Note that this does not include changes only to help content. (see Modifying change log).

  • I adhere to the Command Guidelines.

@shenmuxiaosen
Copy link
Contributor Author

Please add @avanigupta as reviewer. Thanks.

@shenmuxiaosen shenmuxiaosen changed the title Fix appconfig pagination and globalization bugs. [AppConfig]Fix pagination and globalization bugs. Dec 10, 2019
@avanigupta
Copy link
Member

    url = 'https://{}{}'.format(endpoint, query_url)

Do we not need to append api version here? Same for unlock and delete urls.


Refers to: src/azure-cli/azure/cli/command_modules/appconfig/_azconfig/azconfig_client.py:319 in f75c1d3. [](commit_id = f75c1d3, deletion_comment = False)

@avanigupta
Copy link
Member

def delete_keyvalue_by_key_label(self, key, label=None, modify_options=None):

Looks like dead code. Can you remove it please?


Refers to: src/azure-cli/azure/cli/command_modules/appconfig/_azconfig/azconfig_client.py:153 in f75c1d3. [](commit_id = f75c1d3, deletion_comment = False)

@shenmuxiaosen
Copy link
Contributor Author

def delete_keyvalue_by_key_label(self, key, label=None, modify_options=None):

This is part of our python SDK. We have two methods to delete key values. delete_keyvalue(keyvalue) will check etag and fail if mismatch. For delete_keyvalue_by_key_label(key, label) it is a force delete that won't check pre condition. This may not be used in CLI, but we should keep it in internal python sdk. We may consider to use the official python SDK once it is stabilized.


In reply to: 564198076 [](ancestors = 564198076)


Refers to: src/azure-cli/azure/cli/command_modules/appconfig/_azconfig/azconfig_client.py:153 in f75c1d3. [](commit_id = f75c1d3, deletion_comment = False)

@shenmuxiaosen
Copy link
Contributor Author

    url = 'https://{}{}'.format(endpoint, query_url)

yes, good catch. I will add it. Same with unlock and delete.


In reply to: 564196480 [](ancestors = 564196480)


Refers to: src/azure-cli/azure/cli/command_modules/appconfig/_azconfig/azconfig_client.py:319 in f75c1d3. [](commit_id = f75c1d3, deletion_comment = False)

Copy link
Member

@avanigupta avanigupta left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@Juliehzl Juliehzl self-requested a review December 12, 2019 01:49
@Juliehzl Juliehzl self-assigned this Dec 12, 2019
@Juliehzl
Copy link
Contributor

Hi @shenmuxiaosen, please update history.rst for your change.

@Juliehzl
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Juliehzl Juliehzl merged commit 78e201f into Azure:dev Dec 12, 2019
@shenmuxiaosen shenmuxiaosen deleted the user/shuai/pagination branch May 4, 2020 22:43
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.

3 participants