power bi api wrapper integration tests & bug fix

[aditya-pethe]

Powerbi API wrapper bug fix + integration tests

• Bug fix by removing TYPE_CHECKING in in utilities/powerbi.py
• Added integration test for power bi api in utilities/test_powerbi_api.py
• Added integration test for power bi agent in agent/test_powerbi_agent.py
• Edited .env.examples to help set up power bi related environment variables
• Updated demo notebook with working code in docs../examples/powerbi.ipynb - AzureOpenAI -> ChatOpenAI

Notes:

Chat models (gpt3.5, gpt4) are much more capable than davinci at writing DAX queries, so that is important to getting the agent to work properly. Interestingly, gpt3.5-turbo needed the examples=DEFAULT_FEWSHOT_EXAMPLES to write consistent DAX queries, so gpt4 seems necessary as the smart llm.

Fixes #4325

Before submitting

Azure-core and Azure-identity are necessary dependencies

check integration tests with the following:
pytest tests/integration_tests/utilities/test_powerbi_api.py
pytest tests/integration_tests/agent/test_powerbi_agent.py

You will need a power bi account with a dataset id + table name in order to test. See .env.examples for details.

Who can review?

@hwchase17
@vowelparrot

[flipg1995]

Eager to test this fix!

[aditya-pethe]

@eyurtsev bump - I added a commit that should pass linting / make test, it looks like some azure dependencies needed to be included in unit tests. Thanks for the initial workflow run

[eyurtsev]

Suggested change

	azure-core = "^1.26.4"
	azure-core = "^1.26.4"

[eyurtsev]

Suggested change

	azure-identity = "^1.12.0"
	azure-identity = "^1.12.0"

[eyurtsev]

This change makes it such that the code breaks on import for all users that don't have azure installed (and azure will not be installed for most users as it is an optional dependency).

I outlined changes to the poetry file below that will help specify the optionality of the dependency correctly

[eyurtsev]

@aditya-pethe I outlined some changes to the poetry file. The goal is to treat all these dependencies as optional, so the module code should not assume that the dependency is installed. Could you add a unit-test that attempts to import the module. We'll want to make sure that the unit-test passes with the default main group.

I'm working on improving how we handle tests, since the workflow right now is pretty confusing (it's possible to get tests to pass, but have the package break on installs for users)

[aditya-pethe]

@eyurtsev thanks for the feedback! I have a few questions, but to my understanding I should:

• remove azure-identity = "^1.12.0" and azure-core = "^1.26.4" changes I made to the pyproject.toml
• remove the mandatory import from azure.core.credentials import TokenCredential from powerbi.py
• add a test, possibly in tests/integration_tests/utilities/test_powerbi_api.py, which attempts to import azure.core.credentials

Earlier, azure.core.credentials was under TYPE_CHECKING, which caused our original bug. I think the best way to handle this could be modeled after the existing duck duck go tool. In duckduckgo_search.py, the optional module ddg is imported within a try catch, and has a corresponding unit test for importing as well. I.e

        try:
            from duckduckgo_search import ddg  # noqa: F401
        except ImportError:
            raise ValueError(
                "Could not import duckduckgo-search python package. "
                "Please install it with `pip install duckduckgo-search`."
            )

I think I'll use this same logic for powerbi if that looks good. I'll essentially restore the pyproject.toml file to normal and add some try-catch logic to the import, along with an import unit test.

[eyurtsev]

add a test, possibly in tests/integration_tests/utilities/test_powerbi_api.py, which attempts to import azure.core.credentials

You can place the test under tests/unit_tests/ path instead of tests/integration_tests. This will get CI to pick up the test.

  try:
       from duckduckgo_search import ddg  # noqa: F401
   except ImportError:
       raise ValueError(
           "Could not import duckduckgo-search python package. "
           "Please install it with `pip install duckduckgo-search`."
       )

This style works as long as it doesn't raise the ValueError in the global namespace. The file should be ideally safe to import even if the underlying dependency isn't installed.

[aditya-pethe]

Thanks @eyurtsev
The new commit handles changes the following way:

• pyproject.toml is restored to normal (no az packages in test)
• both integration tests will skip the test_daxquery() function if the necessary azure packages are not installed
• the powerbi.py file imports the azure package in a try catch, and just raises a warning if there is an exception
• there is a new powerbi unit test under unit_tests/tools, which just imports the module from powerbi.py to make sure it works

I tested this with make lint / make test with no az packages installed, so this should pass CI. I also checked the behavior of tests with az packages installed - i.e it correctly skips if they are not installed, and passes if they are.

[aditya-pethe]

If all looks fine @eyurtsev we can merge and I can close the PR - not sure if we need another maintainer for that. Thanks!

[eyurtsev]

Hi @aditya-pethe it looks great. We still need to make one change to the unit test so the test verifies that the code can be imported with azure dependency missing. Feel free to make the change yourself, or else one of the maintainers will make it over the next few days.

[eyurtsev]

#4983

[eyurtsev]

was merged #4983