dataops-tk / tap-powerbi-metadata

tap-powerbi-metadata
MIT License
5 stars 2 forks source link

Upgrade SDK version to v0.1.0 #23

Closed aaronsteers closed 3 years ago

aaronsteers commented 3 years ago

@jtimeus-slalom - I hope you don't mind I started the work to migrate from the pre-release SDK version over to the latest.

Here are my steps:

  1. poetry add singer-sdk=="^0.1.0"
  2. import the new standard tests file from the cookiecutter template
  3. remove references to helpers module which is now deprecated
    1. utcnow() is deprecated. It was being imported but wasn't being referenced so I simply deleted the import.
    2. singer_sdk.helpers.typing is now simply singer_sdk.typing. I removed .helpers from the applicable reference lines.

Ready for retest!

poetry run pytest should do the trick.

jtimeus-slalom commented 3 years ago

poetry run pytest is throwing a strange error for me. I got the tap working after one method name change, and confirmed that data was being emitted in a format suitable for target consumption, but I don't understand the error coming from pytest

PS C:\Repos_Other\tap-powerbi-metadata> poetry run tap-powerbi-metadata --config=.secrets\config.json > Activity.jsonl

time=2021-04-12 15:57:09 name=tap-powerbi-metadata level=INFO message=tap-powerbi-metadata v0.0.1, Singer SDK v0.1.2)
time=2021-04-12 15:57:09 name=tap-powerbi-metadata level=INFO message=Skipping parse of env var settings...
time=2021-04-12 15:57:09 name=tap-powerbi-metadata level=INFO message=Config validation passed with 0 errors and 0 warnings.
time=2021-04-12 15:57:09 name=tap-powerbi-metadata level=INFO message=Beginning INCREMENTAL sync of stream 'ActivityEvents'...
time=2021-04-12 15:57:09 name=tap-powerbi-metadata level=INFO message=OAuth authorization attempt was successful.
time=2021-04-12 15:57:11 name=tap-powerbi-metadata level=INFO message=OAuth authorization attempt was successful.
time=2021-04-12 15:57:13 name=tap-powerbi-metadata level=INFO message=OAuth authorization attempt was successful.

Aborted!

PS C:\Repos_Other\tap-powerbi-metadata> cat Activity.jsonl | target-csv

INFO Sending version information to singer.io. To disable sending anonymous usage data, set the config parameter "disable_collection" to true

PS C:\Repos_Other\tap-powerbi-metadata> poetry run pytest

  FileNotFoundError

  [WinError 2] The system cannot find the file specified

  at c:\python38\lib\subprocess.py:1307 in _execute_child
      1303│             sys.audit("subprocess.Popen", executable, args, cwd, env)
      1304│ 
      1305│             # Start the process
      1306│             try:
    → 1307│                 hp, ht, pid, tid = _winapi.CreateProcess(executable, args,
      1308│                                          # no special security
      1309│                                          None, None,
      1310│                                          int(not close_fds),
      1311│                                          creationflags,
PS C:\Repos_Other\tap-powerbi-metadata>

Maybe it has something to do with poetry expecting an exe as noted here?

I haven't figure out how to inspect what is actually being passed in to _execute_child causing the error

jtimeus-slalom commented 3 years ago

well, it helps of pytest is installed...did a pipx install pytest and got further

Now poetry run pytest is giving an error about not being able to import singer_sdk, even though there are several import singer_sdk statements in the code.

PS C:\Repos_Other\tap-powerbi-metadata> poetry run pytest
========================================================== test session starts ==========================================================
platform win32 -- Python 3.8.6, pytest-6.2.3, py-1.10.0, pluggy-0.13.1
rootdir: C:\Repos_Other\tap-powerbi-metadata
collected 0 items / 1 error

================================================================ ERRORS =================================================================
__________________________________ ERROR collecting tap_powerbi_metadata/tests/test_standard_tests.py ___________________________________ 
ImportError while importing test module 'C:\Repos_Other\tap-powerbi-metadata\tap_powerbi_metadata\tests\test_standard_tests.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
c:\python38\lib\importlib\__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
tap_powerbi_metadata\tests\test_standard_tests.py:5: in <module>
    from singer_sdk.testing import get_standard_tap_tests
E   ModuleNotFoundError: No module named 'singer_sdk'
======================================================== short test summary info ======================================================== 
ERROR tap_powerbi_metadata/tests/test_standard_tests.py
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! 
=========================================================== 1 error in 0.12s ============================================================ 
PS C:\Repos_Other\tap-powerbi-metadata> 
aaronsteers commented 3 years ago

@jtimeus-slalom - hello!

Now poetry run pytest is giving an error about not being able to import singer_sdk, even though there are several import singer_sdk statements in the code.

This makes me think that pytest is not aware of the poetry environment. I checked and pytest is included in pyproject.toml. Can you try to run poetry install just in case that pytest library hasn't been installed yet in the poetry virtual environment? Also, in case that doesn't work by itself, it might be worthwhile to uninstall (at least temporarily for debugging) the global version of pytest (pipx uninstall pytest).

In the meanwhile, I'll see if I can replicate the issue locally.


Update: On my side, I'm not seeing the same error - but poetry install was necessary to make it run. I was able to replicate the No module named 'singer_sdk' by uninstalling pytest from the virtual environment (poetry remove -D pytest). That issue also persisted if I reverted the pyproject.toml and poetry.lock files, and was resolved (for me at least) by rerunning poetry install. This makes me more optimistic that hopefully poetry install will do the trick for you also. Let me know if not. Thanks!

aaronsteers commented 3 years ago

Regarding this error:


  FileNotFoundError

  [WinError 2] The system cannot find the file specified

I can confirm this looks like a poetry error, most likely poetry failing to find pytest. (As discussed above in this thread.)

aaronsteers commented 3 years ago

@jtimeus-slalom - I saw the latest commit of adding pytest to CI. It looks like it's running them and just needing env vars. 👍

I don't know if I mentioned this as a breaking change earlier, but the SDK now looks for env variables in the form (all uppercase): <PLUGIN>_<SETTING>. (Previously, the expected input was a lowercase setting.)

Also:

jtimeus-slalom commented 3 years ago

I think I might have triggered an infosec tool that blocks suspicious access, but after doing the poetry remove -D pytest then reverting the files and doing poetry install a few times, it started running the tests...? I am now getting errors like the ones seen in the cicd pipeline output

jtimeus-slalom commented 3 years ago

Was able to get pytest to succeed locally by updating test_standard_tests.py to read from my local config file

PS C:\Repos_Other\tap-powerbi-metadata> poetry run pytest
========================================================== test session starts ========================================================== 
platform win32 -- Python 3.8.6, pytest-6.2.3, py-1.10.0, pluggy-0.13.1
rootdir: C:\Repos_Other\tap-powerbi-metadata
collected 1 item                                                                                                                          

tap_powerbi_metadata\tests\test_standard_tests.py .                                                                                [100%] 

=========================================================== 1 passed in 1.81s =========================================================== 
PS C:\Repos_Other\tap-powerbi-metadata>

This won't allow us to run pytest in the cicd pipeline until we have a test org or some other safe location to auth to and test against, so I commented that step of the pipeline back to the way it was