10000 prepare utils for pypi distribution by leahecole · Pull Request #5848 · GoogleCloudPlatform/python-docs-samples · GitHub
[go: up one dir, main page]

Skip to content

prepare utils for pypi distribution #5848

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

Merged
merged 3 commits into from
May 19, 2021
Merged

Conversation

leahecole
Copy link
Collaborator
@leahecole leahecole commented May 19, 2021

Description

I redid the setup.py with a better, more descriptive name, and added a pyproject.toml. I haven't added it to PyPI yet in case we wanted to do something like change the name, but to test locally, you can

  • Change to this branch and to composer/dag_test_utils
  • Build with python3 -m build (you might have to pip install build)
  • Change to ../workflows
  • pip install ../dag_test_utils/dist/cloud_composer_dag_test_utils-0.0.1.tar.gz
  • Open a python editor and try import internal_unit_testing - you may see some deprecation warnings, and those are okay. They're part of underlying Airflow and will be resolved in the next release. If you see any ModuleNotFoundError errors, we should talk because I probably got something wrong.
  • (alternative path) go to the requirements-test.txt and replace the gross URL with ../dag_test_utils/dist/cloud_composer_dag_test_utils-0.0.1.tar.gz, then run nox -s py-3.8 -- quickstart_test.py (or any of the tests, or the whole test suite - I just suggest doing one for speed reasons because the pip install of Airflow is slow)

I have requested a pywindows cloudtop and will test these distributions on windows and linux as well, and I have properly registered a team account on PyPI for the Cloud Composer DPEs per the OSPO instructions.

Preview of Coming Attractions
Once this is merged, I'll

  • Make a small PR to requirements-test.txt files under Composer so that it pulls this package not a gross URL
  • Make a PR with a major version update that upgrades this code to Airflow 2.0
  • Point PR cleanup: Upgrade Composer samples to Airflow 2.0 #5782 to that new major version upgrade

Note: It's a good idea to open an issue first for discussion.

Checklist

@leahecole leahecole requested a review from a team as a code owner May 19, 2021 18:22
@product-auto-label product-auto-label bot added the samples Issues that are directly related to samples. label May 19, 2021
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label May 19, 2021
@leahecole leahecole force-pushed the package-dag-test-utils branch from eb3c973 to b2c68d6 Compare May 19, 2021 18:24
@leahecole leahecole added the api: composer Issues related to the Cloud Composer API. label May 19, 2021
@@ -6,7 +6,7 @@ This package is used internally to unit test the validity of all Cloud Composer

Add the following to your `requirements-test.txt` file:

`git+https://github.com/GoogleCloudPlatform/python-docs-samples.git#egg=dag_test_utils&subdirectory=composer/dag_test_utils`
`cloud_composer_dag_test_utils`
Copy link
Contributor

Choose a reason for hiding this comment

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

should google be in the package name or is this agnostic to our stuffs?

Copy link
Collaborator Author
@leahecole leahecole May 19, 2021

Choose a reason for hiding this comment

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

At the moment, it's agnostic to our stuff, though I could envision it being expanded to be more composer-y in the future. I wasn't sure if there were restrictions on using google in the package name - I'm happy to add it if you think it's appropriate here

Copy link
Contributor

Choose a reason for hiding this comment

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

I could go either way on this. I tend to think being upfront that "this is a google package" is an alright thing, but could also think that by not including google in the name it is more likely to be 'hidden' when folks search for things. :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Okay! I think I'll keep it as is for now for that 'hidden' reason

@leahecole leahecole merged commit 23f7dba into master May 19, 2021
@leahecole leahecole deleted the package-dag-test-utils branch May 19, 2021 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: composer Issues related to the Cloud Composer API. cla: yes This human has signed the Contributor License Agreement. samples Issues that are directly related to samples.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0