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

Replace config with _config in azure_rm_common.py to support azure-mgmt-network latest version #904

Merged
merged 1 commit into from
Jul 20, 2022

Conversation

saito-hideki
Copy link
Contributor

@saito-hideki saito-hideki commented Jun 29, 2022

SUMMARY

In azure-mgmt-network version 16.0.0 (or later), config seems to have been replaced with _config. config is only used in some logic, but it raises an AttributeError, especially when cert_validation_mode: ignore is specified.
This PR replaces config with _config to address this matter.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME
  • plugins/module_utils/azure_rm_common.py
ADDITIONAL INFORMATION

None

@saito-hideki saito-hideki changed the title [WIP] Replace config with _config for NetworkManagementClient Replace config with _config for NetworkManagementClient Jun 29, 2022
@saito-hideki saito-hideki changed the title Replace config with _config for NetworkManagementClient Replace config with _config in azure_rm_common.py to support azure-mgmt-network latest version Jun 29, 2022
@saito-hideki saito-hideki marked this pull request as ready for review June 29, 2022 10:33
@Fred-sun
Copy link
Collaborator

@saito-hideki The Track2 SDK configuration parameter is ‘_config’, and the Track1 configuration parameter is ‘config’. Thank you very much!

@Fred-sun Fred-sun added question Further information is requested medium_priority Medium priority work in In trying to solve, or in working with contributors labels Jun 30, 2022
@saito-hideki
Copy link
Contributor Author

@Fred-sun Thanks for your advice! :)
I'm new to azure python SDK, so I had replaced config with _config for Track1, but I reverted back. And only replaced it for Track2 like below:

        if not is_track2:
            client.config = self.add_user_agent(client.config)

        if not is_track2 and self.azure_auth._cert_validation_mode == 'ignore':
            client.config.session_configuration_callback = self._validation_ignore_callback
        else:
            client._config.session_configuration_callback = self._validation_ignore_callback

Please let me know if I'm wrong about them.

@Fred-sun
Copy link
Collaborator

Fred-sun commented Jul 4, 2022

@saito-hideki The proposal presses the following way to change, can better. Thank you very much!

        if not is_track2:
            client.config = self.add_user_agent(client.config)
            if self.azure_auth._cert_validation_mode == 'ignore':
                client.config.session_configuration_callback = self._validation_ignore_callback
        else:
            if self.azure_auth._cert_validation_mode == 'ignore':
                client._config.session_configuration_callback = self._validation_ignore_callback

* Addresses issue ansible-collections#903

Signed-off-by: Hideki Saito <saito@fgrep.org>
@saito-hideki
Copy link
Contributor Author

@Fred-sun Thank you for your advice!
I have fixed this PR according to your advice #904 (comment)
I would appreciate it if you could review it :)

@Fred-sun Fred-sun added ready_for_review The PR has been modified and can be reviewed and merged and removed work in In trying to solve, or in working with contributors labels Jul 5, 2022
@xuzhang3
Copy link
Collaborator

LGTM

@xuzhang3 xuzhang3 merged commit 3c2f1b3 into ansible-collections:dev Jul 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
medium_priority Medium priority question Further information is requested ready_for_review The PR has been modified and can be reviewed and merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

azure_rm_privateendpoint failed with AttributeError if cert_validation_mode: ignore specified
3 participants