Skip to content

workers: revision worker implementation - #224

Draft
zzzeid wants to merge 1 commit into
mainfrom
zeid/bug-1744327-refactor-workers
Draft

workers: revision worker implementation#224
zzzeid wants to merge 1 commit into
mainfrom
zeid/bug-1744327-refactor-workers

Conversation

@zzzeid

@zzzeid zzzeid commented Nov 15, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

NOTE: original branch name references the wrong bug.

WIP DO NOT MERGE

  • implement base RevisionWorker (bug 1788728)
  • implement Supervisor worker (bug 1835861)
  • implement Processor worker (bug 1835862)
  • add repo.use_revision_worker feature flag (bug 1788732)
  • add main worker flag and capacity/throttle flags
  • add method to parse diff and list affected files
  • add test coverage for revision_worker.py
  • add new start/stop commands to manage workers
  • add new flags to stop workers gracefully (*_WORKER_STOPPED)
  • refactor dependency and stack fetching and parsing using networkx
  • rename old command lando-cli landing-worker to lando-cli start-landing-worker

TODO:

  • detect stack change on page load
  • add tests for new warnings

@zzzeid zzzeid mentioned this pull request Nov 15, 2022
Closed
@zzzeid
zzzeid marked this pull request as ready for review November 15, 2022 14:41
@zzzeid
zzzeid changed the base branch from main to zeid/bug-1791807-upgrade-cache November 15, 2022 14:45
@zzzeid
zzzeid force-pushed the zeid/bug-1744327-refactor-workers branch 3 times, most recently from 61a474e to 306ca99 Compare November 16, 2022 14:16
zzzeid
zzzeid commented Nov 16, 2022
View reviewed changes
Comment thread landoapi/api/transplants.py Outdated
@zzzeid zzzeid mentioned this pull request Nov 22, 2022
Merged
zzzeid
zzzeid commented Nov 24, 2022
View reviewed changes
Comment thread landoapi/app.py Outdated
zzzeid
zzzeid commented Nov 24, 2022
View reviewed changes
Comment thread landoapi/app.py Outdated
zzzeid
zzzeid commented Nov 24, 2022
View reviewed changes
Comment thread landoapi/app.py Outdated
zzzeid
zzzeid commented Nov 24, 2022
View reviewed changes
Comment thread landoapi/app.py Outdated
zzzeid
zzzeid commented Nov 24, 2022
View reviewed changes
Comment thread landoapi/app.py Outdated
zzzeid
zzzeid commented Nov 24, 2022
View reviewed changes
Comment thread landoapi/models/landing_job.py Outdated
zzzeid
zzzeid commented Nov 24, 2022
View reviewed changes
Comment thread landoapi/models/revisions.py Outdated
zzzeid
zzzeid commented Nov 24, 2022
View reviewed changes
Comment thread landoapi/models/revisions.py Outdated
zzzeid
zzzeid commented Nov 24, 2022
View reviewed changes
Comment thread landoapi/workers/landing_worker.py Outdated
zzzeid
zzzeid commented Nov 24, 2022
View reviewed changes
Comment thread landoapi/workers/revision_worker.py Outdated
zzzeid
zzzeid commented Nov 24, 2022
View reviewed changes
Comment thread landoapi/workers/revision_worker.py Outdated
zzzeid
zzzeid commented Nov 24, 2022
View reviewed changes
Comment thread landoapi/workers/revision_worker.py Outdated
cgsheeh
cgsheeh requested changes Nov 24, 2022
View reviewed changes
Comment thread landoapi/api/revisions.py Outdated
Comment thread landoapi/api/transplants.py Outdated
Comment thread landoapi/models/landing_job.py Outdated
Comment thread landoapi/models/revisions.py Outdated
Comment thread landoapi/models/revisions.py
Comment thread landoapi/workers/revision_worker.py Outdated
Comment thread landoapi/workers/revision_worker.py Outdated
Comment thread landoapi/workers/revision_worker.py
Comment on lines +353 to +320
try:
warnings, errors = self.process(revision)
except Exception as e:
logger.info(f"Exception encountered while processing {revision}")
revision.status = RS.PROBLEM
revision.update_data(error="".join(e.args))
logger.exception(e)
db.session.commit()
continue

@cgsheeh cgsheeh Nov 24, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we should catch a different exception here.

@zzzeid zzzeid Dec 2, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I will look into this one.

Comment thread landoapi/workers/revision_worker.py Outdated
Comment thread tests/test_sanitized_commit_messages.py Outdated
@zzzeid
zzzeid requested a review from cgsheeh December 2, 2022 14:34
cgsheeh
cgsheeh requested changes Dec 14, 2022
View reviewed changes

@cgsheeh cgsheeh left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A few more comments, questions etc.

Comment thread landoapi/api/transplants.py Outdated
Comment thread landoapi/api/transplants.py Outdated
Comment thread landoapi/api/transplants.py Outdated
Comment thread landoapi/api/transplants.py Outdated
Comment thread landoapi/api/transplants.py Outdated
Comment thread landoapi/cli.py
Comment thread landoapi/models/landing_job.py Outdated
Comment thread landoapi/models/landing_job.py Outdated
Comment thread landoapi/models/revisions.py Outdated
Comment thread landoapi/projects.py Outdated
cgsheeh
cgsheeh requested changes Dec 22, 2022
View reviewed changes

@cgsheeh cgsheeh left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please don't use single letter variable names.

Comment thread landoapi/stacks.py Outdated
Comment thread landoapi/workers/base.py
Comment thread landoapi/workers/revision_worker.py
Comment on lines +69 to +72
statuses = statuses or [
"draft",
"needs-review",
"accepted",
"published",
"changes-planned",
]

@cgsheeh cgsheeh Dec 17, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is the significance of this list as the default? If you could give this a :sort too that would be great.

@zzzeid zzzeid Feb 27, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It limits how many revisions we are synchronizing based on those that we need. The list will probably be tweaked in the future (e.g., maybe we don't sync drafts, or changes-planned, since those won't be landable anyway.)

Comment thread landoapi/workers/revision_worker.py Outdated
Comment thread landoapi/workers/revision_worker.py Outdated
Comment thread landoapi/workers/revision_worker.py
Comment thread landoapi/models/revisions.py Outdated
Comment thread landoapi/workers/revision_worker.py Outdated
Comment thread landoapi/workers/revision_worker.py Outdated
Comment thread landoapi/workers/revision_worker.py Outdated
@zzzeid
zzzeid force-pushed the zeid/bug-1791807-upgrade-cache branch from c1be7fe to 47de8d0 Compare February 28, 2023 14:45
@zzzeid
zzzeid force-pushed the zeid/bug-1791807-upgrade-cache branch 2 times, most recently from 9fea31a to 295c589 Compare March 13, 2023 12:29
Base automatically changed from zeid/bug-1791807-upgrade-cache to main March 13, 2023 12:44
@zzzeid
zzzeid force-pushed the zeid/bug-1744327-refactor-workers branch from bd0c611 to c3ec081 Compare March 22, 2023 01:17
@zzzeid
zzzeid marked this pull request as draft March 22, 2023 12:12
zzzeid
zzzeid commented Apr 27, 2023
View reviewed changes
Comment thread Dockerfile Outdated
zzzeid
zzzeid commented Apr 27, 2023
View reviewed changes
Comment thread Dockerfile-dev Outdated
@zzzeid
zzzeid force-pushed the zeid/bug-1744327-refactor-workers branch 4 times, most recently from fcfd98b to 30b64e9 Compare May 11, 2023 19:02
@zzzeid
zzzeid force-pushed the zeid/bug-1825277-landing-job-revisions branch 4 times, most recently from 0c15645 to e093d5f Compare May 17, 2023 19:24
Base automatically changed from zeid/bug-1825277-landing-job-revisions to main May 17, 2023 19:30
@zzzeid
zzzeid force-pushed the zeid/bug-1744327-refactor-workers branch 5 times, most recently from 3225339 to 8e73e5a Compare May 24, 2023 19:12
@zzzeid
zzzeid force-pushed the zeid/bug-1744327-refactor-workers branch 4 times, most recently from 00ba1c3 to f3ededa Compare May 29, 2023 17:27
@zzzeid
zzzeid changed the base branch from main to zeid/bug-1835678-add-lando-revision-to-api May 29, 2023 17:28
@zzzeid
zzzeid force-pushed the zeid/bug-1744327-refactor-workers branch 2 times, most recently from 0e58aeb to 18abe99 Compare May 29, 2023 17:45
Base automatically changed from zeid/bug-1835678-add-lando-revision-to-api to main May 29, 2023 20:35
@zzzeid
zzzeid force-pushed the zeid/bug-1744327-refactor-workers branch 3 times, most recently from 0fb50f6 to 5240202 Compare May 30, 2023 19:24
zzzeid
zzzeid commented May 30, 2023
View reviewed changes
Comment thread landoapi/workers/revision_worker.py
for r in revision.predecessors + [revision]:
try:
hgrepo.apply_patch(io.BytesIO(r.patch_bytes))
except Exception as e:

@zzzeid zzzeid May 30, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

See this comment.

@zzzeid
workers: revision worker implementation
dd677b6 
NOTE: original branch name references the wrong bug.

WIP DO NOT MERGE

- implement base RevisionWorker (bug 1788728)
- implement Supervisor worker (bug 1835861)
- implement Processor worker (bug 1835862)
- add repo.use_revision_worker feature flag (bug 1788732)
- add main worker flag and capacity/throttle flags
- add method to parse diff and list affected files <*******
- add test coverage for revision_worker.py
- add new start/stop commands to manage workers
- add new flags to stop workers gracefully (*_WORKER_STOPPED)
- refactor dependency and stack fetching and parsing using networkx <********
- rename old command lando-cli landing-worker to lando-cli start-landing-worker

TODO:
- detect stack change on page load
- add tests for new warnings
@zzzeid
zzzeid force-pushed the zeid/bug-1744327-refactor-workers branch from 5240202 to dd677b6 Compare May 31, 2023 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cgsheeh cgsheeh cgsheeh requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@zzzeid @cgsheeh