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

Improve git change detection logic #174

Merged
merged 26 commits into from
Aug 10, 2023

Conversation

SamTheisens
Copy link
Collaborator

@SamTheisens SamTheisens commented Jul 31, 2023

Attempts to make the change detection logic on a PR more robust.

  • Improve build status output, making it more instructive
  • Remove ad-hoc pull in "determine changes on branch", because it smells like voodoo cult. If the state of the branch is not the way it should be we should exit with a warning, not try to fix it and muddle on.
  • Allow for Jenkins to not be configured for scenarios in which another build tool is used
  • Allow for minimal clone scenarios in which the default branch is not present (Is it possible to checkout the whole history since the branch creation? actions/checkout#266 (comment))
  • Create repo command to optionally bring the branch into the required state for a PR

Also fixes a confusing problem in Jenkinsfile. The mpyl_config needs to be pulled from the Vandebron/mpyl_config repo. This created a branch called main in the local .git folder that pointed to a revision that doesn't exist in the Vandebron/mpyl repo. As a result we compare to a base that doesn't exist.
Now the file is curled in from via the github api instead of by checking out the repo.

@Jorg88, it looks like I can't edit the github actions in this repo anymore.
Suggesting to put this

New release `${{ steps.findPr.outputs.pr }}.${{ github.run_number }}` deployed at [Test Pypi](https://test.pypi.org/project/mpyl/). 
Install with `pip install -i https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple mpyl==${{ steps.findPr.outputs.pr }}.${{ github.run_number }}`

at line 156 of .github/workflows/build-package.yml

Test scenario

  1. New branch gco -b feature/test-174

  2. mpyl build status:

    Branch not specified at build.versioning.branch in run_properties.yml. Branch determined via git: feature/test-174
    Branch: feature/test-174 at 96b6be3d121ff1651506da3d6330f7df13c7a959

  3. Make change to projects/service/src/sum.js

    Branch: feature/test-174 at 96b6be3d121ff1651506da3d6330f7df13c7a959
    Execution plan:
    🏗️ nodeservice
    📋 nodeservice
    🚀 nodeservice

  4. Commit change

    Branch: feature/test-174 at e9ff18931070de4803da2190274d5fccb0362824
    Execution plan:
    🏗️ nodeservice
    📋 nodeservice
    🚀 nodeservice

  5. Back to main branch

    1. Make change to sbtservice on main and commit.
    2. gco feature/test-174
    3. git merge main
      Branch: feature/test-174 at 9215d9c863c1fbf842de46bda189c92fa2226716
      Execution plan:
      🏗️ nodeservice
      📋 nodeservice
      🚀 nodeservice

⚠️ Could not find ticket corresponding to `feature/TECH-XXX-fix-change-detection-logic. Does your branch name follow the correct pattern?

@github-actions
Copy link

github-actions bot commented Jul 31, 2023

Code Coverage

Package Line Rate Health
src.mpyl 94%
src.mpyl.cli 66%
src.mpyl.projects 94%
src.mpyl.reporting 100%
src.mpyl.reporting.formatting 96%
src.mpyl.stages 86%
src.mpyl.steps 94%
src.mpyl.steps.build 82%
src.mpyl.steps.deploy 78%
src.mpyl.steps.deploy.k8s 86%
src.mpyl.steps.deploy.k8s.resources 96%
src.mpyl.steps.postdeploy 100%
src.mpyl.steps.test 100%
src.mpyl.utilities 100%
src.mpyl.utilities.cypress 93%
src.mpyl.utilities.github 71%
src.mpyl.utilities.jenkins 69%
src.mpyl.utilities.junit 93%
src.mpyl.utilities.logging 71%
src.mpyl.utilities.pyaml_env 100%
src.mpyl.utilities.repo 77%
src.mpyl.utilities.s3 48%
src.mpyl.utilities.sbt 96%
src.mpyl.utilities.subprocess 78%
Summary 84% (1996 / 2364)

Minimum allowed line rate is 82%

@github-actions
Copy link

github-actions bot commented Jul 31, 2023

New release 174.2077 deployed at Test Pypi.
Install with

pip install -i https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple mpyl==174.2077

@SamTheisens SamTheisens force-pushed the feature/TECH-XXX-fix-change-detection-logic branch from 8649619 to df5792c Compare July 31, 2023 08:06
@Jorg88 Jorg88 self-requested a review July 31, 2023 10:38
@SamTheisens SamTheisens force-pushed the feature/TECH-XXX-fix-change-detection-logic branch 2 times, most recently from 25a3712 to 390c5d2 Compare August 1, 2023 05:52
@SamTheisens SamTheisens marked this pull request as ready for review August 1, 2023 06:11
@SamTheisens SamTheisens changed the title feature/TECH XXX fix change detection logic Improve git change detection logic Aug 1, 2023
@SamTheisens SamTheisens requested a review from koeves August 1, 2023 06:20
@SamTheisens SamTheisens force-pushed the feature/TECH-XXX-fix-change-detection-logic branch 6 times, most recently from 8d9445a to 7235318 Compare August 2, 2023 12:04
@SamTheisens SamTheisens force-pushed the feature/TECH-XXX-fix-change-detection-logic branch 2 times, most recently from 17fe65a to 76bcb53 Compare August 7, 2023 11:08
@SamTheisens SamTheisens requested a review from Jorg88 August 7, 2023 11:09
because it's too implicit and should be done in advance if at all

branch:
e.g. `git clone --shallow-exclude main --single-branch --branch
feature/test-invalidation-logic
https://github.com/SamTheisens/mpyl-example-gha.git`

in which case there will be no default branch, but the first commit is
the base from which the branch was created

branch:
SamTheisens and others added 10 commits August 7, 2023 19:09
and not all refs, including tags

branch:
this should always be origin/main. If a local `main` is merged into the
current branch, that should not supersede the original base

branch:
to bring the branch into a state such that PRs can be built

branch:
branch: feature/TECH-490-migrate-mpyl-to-python-3.11
with basic implementation of status logic

branch:
@SamTheisens SamTheisens force-pushed the feature/TECH-XXX-fix-change-detection-logic branch 3 times, most recently from ceade9a to 4e6ff4e Compare August 8, 2023 04:25
Copy link
Collaborator

@Jorg88 Jorg88 left a comment

Choose a reason for hiding this comment

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

Nice solution I would say Sam!

@SamTheisens SamTheisens force-pushed the feature/TECH-XXX-fix-change-detection-logic branch from 6f90c03 to aef1a39 Compare August 8, 2023 09:08
@koeves
Copy link
Contributor

koeves commented Aug 8, 2023

I tried running this test version with a real PR in jenkins, it got to

git.exc.GitCommandError: Cmd('git') failed due to: exit code(128)
  cmdline: git describe --tags a6fc2e5d433165644ab29ab1283441dbb9e54b8a
  stderr: 'fatal: No names found, cannot describe anything.'

then based on Jorg's advice added the following to the jenkinsfile:

sh "pipenv run mpyl repo status"
sh "pipenv run mpyl repo init"

which got stuck on Initializing PR-11178...

Maybe it again waits for input in the agent?

EDIT if I try to clone the repo on the agent it asks for the username

@@ -0,0 +1 @@
"""CVS repository related command group"""
Copy link
Collaborator

Choose a reason for hiding this comment

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

did you add the empty init.py file below (I can't comment on it) on purpose btw?

@SamTheisens SamTheisens force-pushed the feature/TECH-XXX-fix-change-detection-logic branch 2 times, most recently from f9dd62b to cfdd777 Compare August 9, 2023 08:31
because it is not secure

branch:
@SamTheisens SamTheisens force-pushed the feature/TECH-XXX-fix-change-detection-logic branch from cfdd777 to c77a4bc Compare August 9, 2023 08:46
@SamTheisens SamTheisens force-pushed the feature/TECH-XXX-fix-change-detection-logic branch from c9969a4 to 1b038be Compare August 10, 2023 02:59
@koeves koeves merged commit ab3a8b4 into main Aug 10, 2023
@koeves koeves deleted the feature/TECH-XXX-fix-change-detection-logic branch August 10, 2023 08:06
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