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

guarantee final pipeline #340

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
* 0.10.1:
- Feature: Guarantee pipeline before merging
* 0.10.0:
- Feature: implement HTTPS support for cloning (#225) #283
- Feature: Make CI work with GitHub Actions #308
Expand Down
9 changes: 8 additions & 1 deletion marge/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def time_interval(str_interval):
) from err


def _parse_config(args):
def _parse_config(args): # pylint: disable=too-many-statements

def regexp(str_regex):
try:
Expand Down Expand Up @@ -237,6 +237,12 @@ def regexp(str_regex):
action='store_true',
help='Run marge-bot as a single CLI command, not a service'
)
parser.add_argument(
'--guarantee-final-pipeline',
action='store_true',
help='Guaranteed final pipeline when assigned to marge-bot'
)

config = parser.parse_args(args)

if config.use_merge_strategy and config.batch:
Expand Down Expand Up @@ -342,6 +348,7 @@ def main(args=None):
use_no_ff_batches=options.use_no_ff_batches,
use_merge_commit_batches=options.use_merge_commit_batches,
skip_ci_batches=options.skip_ci_batches,
guarantee_final_pipeline=options.guarantee_final_pipeline,
),
batch=options.batch,
cli=options.cli,
Expand Down
3 changes: 3 additions & 0 deletions marge/job.py
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,7 @@ class Fusion(enum.Enum):
'use_no_ff_batches',
'use_merge_commit_batches',
'skip_ci_batches',
'guarantee_final_pipeline',
]


Expand All @@ -476,6 +477,7 @@ def default(
add_tested=False, add_part_of=False, add_reviewers=False, reapprove=False,
approval_timeout=None, embargo=None, ci_timeout=None, fusion=Fusion.rebase,
use_no_ff_batches=False, use_merge_commit_batches=False, skip_ci_batches=False,
guarantee_final_pipeline=False,
):
approval_timeout = approval_timeout or timedelta(seconds=0)
embargo = embargo or IntervalUnion.empty()
Expand All @@ -492,6 +494,7 @@ def default(
use_no_ff_batches=use_no_ff_batches,
use_merge_commit_batches=use_merge_commit_batches,
skip_ci_batches=skip_ci_batches,
guarantee_final_pipeline=guarantee_final_pipeline,
)


Expand Down
13 changes: 12 additions & 1 deletion marge/single_merge_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ class SingleMergeJob(MergeJob):
def __init__(self, *, api, user, project, repo, options, merge_request):
super().__init__(api=api, user=user, project=project, repo=repo, options=options)
self._merge_request = merge_request
self._options = options

def execute(self):
merge_request = self._merge_request
Expand Down Expand Up @@ -61,7 +62,17 @@ def update_merge_request_and_accept(self, approvals):
merge_request.comment("Someone skipped the queue! Will have to try again...")
continue

log.info('Commit id to merge %r (into: %r)', actual_sha, target_sha)
if _updated_sha == actual_sha and self._options.guarantee_final_pipeline:
log.info('No commits on target branch to fuse, triggering pipeline...')
merge_request.comment("jenkins retry")

Choose a reason for hiding this comment

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

How does this guarantee that the final pipeline is run? It seems especially suspicious to add a comment saying "jenkins retry" here. Is that some internal smarkets CI stuff that's leaking out here? :)

Copy link

Choose a reason for hiding this comment

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

shouldn't this be a merge_request_pipeline_trigger call instead ? Maybe I'm missing something ...

Copy link
Contributor

Choose a reason for hiding this comment

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

This is essentially what was done in the community fork in this MR: https://gitlab.com/marge-org/marge-bot/-/merge_requests/387.

This upstream project is no longer maintained as far as we can tell.

time.sleep(30)

log.info(
'Commit id to merge %r into: %r (updated sha: %r)',
actual_sha,
target_sha,
_updated_sha
)
time.sleep(5)

sha_now = Commit.last_on_branch(source_project.id, merge_request.source_branch, api).id
Expand Down
1 change: 1 addition & 0 deletions tests/test_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@ def test_default(self):
use_no_ff_batches=False,
use_merge_commit_batches=False,
skip_ci_batches=False,
guarantee_final_pipeline=False,
)

def test_default_ci_time(self):
Expand Down
2 changes: 1 addition & 1 deletion version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
0.10.0
0.10.1