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

Setup the new auto-release CircleCI Release #43

Merged
merged 57 commits into from
Jul 12, 2022
Merged

Conversation

noklam
Copy link
Contributor

@noklam noklam commented Jul 4, 2022

Description

To adapt the monorepo structure and support auto-release for plugin

The old process

  1. Every time main is updated -> Check if __version__.py is updated, if so trigger a sync job
  2. Check if the version is on PyPI - if not - trigger - Send a POST request to CircleCI to trigger publish_kedro pipeline
  3. Twine for PyPI / Another request for GitHub Release

The Current Flow

  1. Whenever main get updated, it checked if any of the repositories need to do a release (By looping all __version__ and compare to PyPI)
  2. Another CircleCI pipeline is triggered with the pipeline parameter (release_version and release_package) which triggers the build and publish workflow.

Development notes

  • release_package & release_version replace the release_kedro :True to handle multiple repositories
  • Refactor and combine a few bash scripts into 1 Python script
  • Put some documentation back into code about how to create the token
  • A successful CI build on CCI
  • Update documentation on Confluence for plugins

The credential on CCI are token generated on PyPI, I don't know where's the best place to document this information, maybe I will document how I do the testing locally on Confluence.

Extra tips:
I only realise I can trigger a pipeline on main with the UI, which should help you to "unit test" some individual workflow. (So you don't have to make a thousand dummy commits to trigger the job 😅)
image

Testings are done locally with Personal Token, we have a separate set of tokens saved in CircleCI - some links are written in docstring to guide how to create these tokens.

Checklist

  • Opened this PR as a 'Draft Pull Request' if it is work-in-progress
  • Updated the documentation to reflect the code changes
  • Added a description of this change in the relevant RELEASE.md file
  • Added tests to cover my changes

noklam added 30 commits June 28, 2022 10:33
Signed-off-by: Nok Chan <[email protected]>
Signed-off-by: Nok Chan <[email protected]>
Signed-off-by: Nok Chan <[email protected]>
Signed-off-by: Nok Chan <[email protected]>
Signed-off-by: Nok Chan <[email protected]>
Signed-off-by: Nok Chan <[email protected]>
Signed-off-by: Nok Chan <[email protected]>
Signed-off-by: Nok Chan <[email protected]>
Signed-off-by: Nok Chan <[email protected]>
This reverts commit eb6ade0.
Signed-off-by: Nok Chan <[email protected]>
Signed-off-by: Nok Chan <[email protected]>
Signed-off-by: Nok Chan <[email protected]>
@@ -0,0 +1 @@
# A dummy file to keep CI behave correctly
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The kedro-datasets didn't really have an e2e test. But keeping a dummy test here simplifies the conditions with CI config and it doesn't have much overhead, so I create a dummy file.

@@ -33,7 +33,7 @@ min-public-methods = 1
[tool.coverage.report]
fail_under = 100
show_missing = true
omit = ["tests/*"]
omit = ["tests/*", "kedro_datasets/datasets/holoviews/*"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Carries from kedro

Signed-off-by: Nok Chan <[email protected]>
Signed-off-by: Nok Chan <[email protected]>
@noklam noklam marked this pull request as ready for review July 7, 2022 12:23
payload = {
"branch": CIRCLE_BRANCH,
"parameters": {
"release_package": package_name,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We used to check the version twice in two different jobs, I think it's better to keep a single source of truth and just pass this information in the requests.

from utils.package_version import get_package_version

PACKAGE_PATHS = (
"kedro-datasets/kedro_datasets",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In theory, we can just use kedro-datasets and parse it, but I am not sure if this is a convention. So I just keep the longer path here, not too important anyway.

}

print(package_name, package_version)
if check_no_version_pypi(pypi_endpoint, package_name, package_version):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Everytime main is updated, it will check all repositories and see if there is a need to do a release. So it is possible that it triggers more than 1 release job (for different repository) but in practice, we shouldn't have one commit changing 2 different repositories.

Signed-off-by: Nok Chan <[email protected]>
@noklam
Copy link
Contributor Author

noklam commented Jul 7, 2022

No idea why getting PermissionError only on Python3.10 + Windows, @MerelTheisenQB any idea? I remember you have fixed some bug in CI that only fails on Python3.10 + Windows+ before.

E PermissionError: Forbidden
../../miniconda/envs/kedro_plugins/lib/python3.10/site-packages/s3fs/core.py:548: PermissionError

Copy link
Member

@merelcht merelcht left a comment

Choose a reason for hiding this comment

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

Some minor questions from me, but in general this looks good to me! 👍

@@ -1,5 +1,13 @@
version: 2.1

parameters:
Copy link
Member

Choose a reason for hiding this comment

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

Why do you need these parameters here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a CircleCI thing, otherwise, it will just throw an error. We called these 2 parameters in the POST request.

Comment on lines 324 to 326
# - run:
# name: Maybe merge main into develop or raise a PR
# command: ./tools/circleci/github_scripts/merge.sh . "main" "develop" "${GITHUB_TAGGING_TOKEN}"
Copy link
Member

Choose a reason for hiding this comment

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

I guess we can remove this as I don't see us adding a develop branch for this repo.

print("Creating CircleCI Pipeline successfully")
print(resp.content)
else:
print("Failed to create CircleCI Pipeline")
Copy link
Member

Choose a reason for hiding this comment

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

Do we need any more info here? Error code or something like that to help us figure out what went wrong?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with Merel, maybe put the response code in the print statement as well.

@merelcht
Copy link
Member

merelcht commented Jul 8, 2022

No idea why getting PermissionError only on Python3.10 + Windows, @MerelTheisenQB any idea? I remember you have fixed some bug in CI that only fails on Python3.10 + Windows+ before.

E PermissionError: Forbidden
../../miniconda/envs/kedro_plugins/lib/python3.10/site-packages/s3fs/core.py:548: PermissionError

Hmm strange.. I think I solved those issues at the time by running tests sequentially instead of in parallel. But it's odd it's just the one test now..

Copy link
Contributor

@SajidAlamQB SajidAlamQB left a comment

Choose a reason for hiding this comment

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

Awesome work on this just a minor comment! 🎉

print("Creating CircleCI Pipeline successfully")
print(resp.content)
else:
print("Failed to create CircleCI Pipeline")
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with Merel, maybe put the response code in the print statement as well.

@noklam
Copy link
Contributor Author

noklam commented Jul 12, 2022

Let's solve this unittest issue for 3.10 separately

@noklam noklam merged commit 5813362 into main Jul 12, 2022
@noklam noklam deleted the feat/refactor-auto-release branch July 12, 2022 15:05
@AhdraMeraliQB AhdraMeraliQB mentioned this pull request Nov 11, 2022
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants