get module name from sys.modules #17779

girarda · 2022-10-10T12:04:43Z

What

This is a fix for #17772.
Acceptance tests don't run from the same main file as running the discovery command.
Instead of inferring the connector's module from the main file, we can get it from the list of modules that are loaded in the environment at runtime

How

Iterate over the modules loaded at runtime and select the one that starts with "source_"

Testing

Testing cdk changes is tedious. I used the following script with the following docker image to run the SATs using this branch

CURRENT_DIR="$PWD"
DIR=$(basename "$PWD")
echo $DIR

# Build SAT
cd ../../bases/source-acceptance-test
docker build . -t airbyte/source-acceptance-test:dev

# Build connector
cd ../../../
docker build . -t airbyte/$DIR:dev -f airbyte-integrations/connectors/$DIR/Dockerfile
cd $CURRENT_DIR
#python3 -m pytest integration_tests -p integration_tests.acceptance

# Run
docker run --rm -it \
    -v /var/run/docker.sock:/var/run/docker.sock \
    -v /tmp:/tmp \
    -v $(pwd):/test_input \
    --env CASSETTE_MODE \
    --env CASSETTE_PATH \
    airbyte/source-acceptance-test:dev \
    --acceptance-test-config /test_input

dockerfile:

FROM python:3.9-slim

# Bash is installed for more convenient debugging.
RUN apt-get update && apt-get install -y bash automake build-essential git libtool libtool-bin autoconf && rm -rf /var/lib/apt/lists/*

WORKDIR /airbyte/integration_code
COPY ./airbyte-integrations/connectors/source-exchange-rates-tutorial-2/source_exchange_rates_tutorial_2 ./source_exchange_rates_tutorial_2
RUN echo "$ls"

COPY ./airbyte-cdk/python ./cdk

COPY ./airbyte-integrations/connectors/source-exchange-rates-tutorial-2/main.py ./
COPY ./airbyte-integrations/connectors/source-exchange-rates-tutorial-2/setup.py ./

#COPY setup.py ./
RUN pip install -e ./cdk
RUN pip install .

ENV AIRBYTE_ENTRYPOINT "python /airbyte/integration_code/main.py"
ENTRYPOINT ["python", "/airbyte/integration_code/main.py"]

LABEL io.airbyte.version=0.3.1
LABEL io.airbyte.name=airbyte/source-exchange-rates-tutorial-2

🚨 User Impact 🚨

Are there any breaking changes? What is the end result perceived by the user? If yes, please merge this PR with the 🚨🚨 emoji so changelog authors can further highlight this if needed.

Pre-merge Checklist

Expand the relevant checklist and delete the others.

New Connector

Community member or Airbyter

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

Create a non-forked branch based on this PR and test the below items on it
Build is successful
If new credentials are required for use in CI, add them to GSM. Instructions.
/test connector=connectors/<name> command is passing
New Connector version released on Dockerhub by running the /publish command described here
After the connector is published, connector added to connector index as described here
Seed specs have been re-generated by building the platform and committing the changes to the seed spec files, as described here

Updating a connector

Community member or Airbyter

Grant edit access to maintainers (instructions)
Secrets in the connector's spec are annotated with airbyte_secret
Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
Code reviews completed
Documentation updated
- Connector's README.md
- Connector's bootstrap.md. See description and examples
- Changelog updated in docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
PR name follows PR naming conventions

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

Create a non-forked branch based on this PR and test the below items on it
Build is successful
If new credentials are required for use in CI, add them to GSM. Instructions.
/test connector=connectors/<name> command is passing
New Connector version released on Dockerhub and connector version bumped by running the /publish command described here

Connector Generator

Issue acceptance criteria met
PR name follows PR naming conventions
If adding a new generator, add it to the list of scaffold modules being tested
The generator test modules (all connectors with -scaffold in their name) have been updated with the latest scaffold by running ./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates then checking in your changes
Documentation which references the generator is updated as needed

Tests

Unit

Put your unit tests output here.

Integration

Put your integration tests output here.

Acceptance

Put your acceptance tests output here.

brianjlai

a few small things, but don't need to block on approval

brianjlai · 2022-10-10T15:10:41Z

airbyte-cdk/python/airbyte_cdk/sources/declarative/schema/json_schema.py

+    # the connector's module name can be inferred by looking at the modules loaded and look for the one starting with source_
+    source_modules = [
+        k for k, v in sys.modules.items() if "source_" in k
+    ]  # example: ['source_exchange_rates', 'source_exchange_rates_tutorial.source']


I'm a little confused by your comment example here. Are you saying that the result of one sys.modules call will result in these two items in the map? Or that these are two possible permutations of how the sys.modules call will be returned (one called from the source_exchange_rates source and one called from the source_exchange_rates_tutorial source)?

fixed the comment. thanks!

brianjlai · 2022-10-10T15:12:54Z

airbyte-cdk/python/airbyte_cdk/sources/declarative/schema/json_schema.py

+    source_modules = [
+        k for k, v in sys.modules.items() if "source_" in k
+    ]  # example: ['source_exchange_rates', 'source_exchange_rates_tutorial.source']
+    module = source_modules[0].split(".")[0]


If a developer went out of their way to name their modules incorrectly w/o source_, this would crash. More of a user error to deviate from the template, but we should check here and throw an error instead of crashing. I'm sure other things would fail if they did that as well, but just in case

girarda · 2022-10-10T15:19:59Z

/publish-cdk dry-run=false

❌ https://github.com/airbytehq/airbyte/actions/runs/3220453525

girarda · 2022-10-10T20:44:06Z

/publish-cdk dry-run=false

🕑 https://github.com/airbytehq/airbyte/actions/runs/3222250101
✅ https://github.com/airbytehq/airbyte/actions/runs/3222250101

* get module name from sys.modules * bump * fix comment * throw exception * fix unittests * Add missing files * remove debug prints * indent

get module name from sys.modules

fa1046b

github-actions bot added the CDK Connector Development Kit label Oct 10, 2022

Merge branch 'master' into alex/infer_module_name

3fe6309

girarda marked this pull request as ready for review October 10, 2022 12:09

girarda requested a review from a team as a code owner October 10, 2022 12:09

girarda requested a review from brianjlai October 10, 2022 12:09

brianjlai approved these changes Oct 10, 2022

View reviewed changes

girarda added 4 commits October 10, 2022 08:16

bump

06ed2ee

fix comment

85a07c2

throw exception

a3c99e9

Merge branch 'master' into alex/infer_module_name

b2fd861

girarda added 5 commits October 10, 2022 10:06

Merge branch 'master' into alex/infer_module_name

9105083

fix unittests

7720a40

Add missing files

8bd0d42

remove debug prints

2822d1f

indent

38d8868

girarda merged commit 62500af into master Oct 10, 2022

girarda deleted the alex/infer_module_name branch October 10, 2022 20:54

This was referenced Oct 13, 2022

Bump Airbyte version from 0.40.14 to 0.40.15 #17961

Closed

Bump Airbyte version from 0.40.14 to 0.40.15 #17970

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

get module name from sys.modules #17779

get module name from sys.modules #17779

girarda commented Oct 10, 2022 •

edited

Loading

brianjlai left a comment

brianjlai Oct 10, 2022

girarda Oct 10, 2022

brianjlai Oct 10, 2022

girarda Oct 10, 2022

girarda commented Oct 10, 2022 •

edited by github-actions bot

Loading

girarda commented Oct 10, 2022 •

edited by github-actions bot

Loading

get module name from sys.modules #17779

get module name from sys.modules #17779

Conversation

girarda commented Oct 10, 2022 • edited Loading

What

How

Recommended reading order

Testing

🚨 User Impact 🚨

Pre-merge Checklist

Community member or Airbyter

Airbyter

Community member or Airbyter

Airbyter

Tests

brianjlai left a comment

Choose a reason for hiding this comment

brianjlai Oct 10, 2022

Choose a reason for hiding this comment

girarda Oct 10, 2022

Choose a reason for hiding this comment

brianjlai Oct 10, 2022

Choose a reason for hiding this comment

girarda Oct 10, 2022

Choose a reason for hiding this comment

girarda commented Oct 10, 2022 • edited by github-actions bot Loading

girarda commented Oct 10, 2022 • edited by github-actions bot Loading

girarda commented Oct 10, 2022 •

edited

Loading

girarda commented Oct 10, 2022 •

edited by github-actions bot

Loading

girarda commented Oct 10, 2022 •

edited by github-actions bot

Loading