Skip to content

Collection API - #3

Merged
alecbcs merged 28 commits into
developfrom
add/collection-func
Feb 12, 2024
Merged

Collection API#3
alecbcs merged 28 commits into
developfrom
add/collection-func

Conversation

@cmelone

@cmelone cmelone commented Dec 25, 2023

Copy link
Copy Markdown
Member

This PR implements a webhook for the status change of Gitlab jobs. Build jobs are selected for processing, where a series of Prometheus queries are issued and data about the build and resource utilization are made available.

Adds: POST endpoint under /collect.

Priorities:

  • Minimize the amount of requests needed to collect the necessary data. At the moment, for builds: 1 request to Gitlab + 5 to Prometheus. For VMs: 2 requests to Prometheus.
  • Handling the potential for duplicates and multiple/erroneous requests for collection. Protections are implemented for race conditions.
  • Composability: API should be readable and potentially reusable for other purposes
  • Speed: while this application is written with async functionality, there is still a requirement to respond to the webhook in under 10 seconds.

All main functionality is implemented and I have removed the draft status from the PR. There are just a couple more things to do:

Also, I realize that this PR is quite big, so I'm happy to break it up into smaller pieces.

@cmelone cmelone self-assigned this Dec 25, 2023
Comment thread gantry/utils/collect.py Outdated
Comment thread gantry/utils/collect.py Outdated
@spack spack deleted a comment from girishbarca Jan 16, 2024
@cmelone cmelone changed the title Add build metadata collection collection webhook Jan 17, 2024
@cmelone cmelone added the feature New feature or request label Jan 18, 2024
@cmelone
cmelone requested a review from alecbcs January 18, 2024 08:33
@cmelone
cmelone marked this pull request as ready for review January 18, 2024 08:33

@alecbcs alecbcs 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.

Super nice job! This PR looks great. Just a couple points of feedback.

I also might suggest creating the sqlite db in the root of the repo and actually putting db/ under gantry/ instead of leaving it as its own top level component. This might fit a bit better depending on how we structure the components. I'd probably go for something like the following, but feel free to push back if you don't find that as easy to work with.

gantry/
  db/
     get.py
     insert.py
  clients/
    prometheus.py
    gitlab.py
  models/
    jobs.py
    vms.py
  routes/
    collection.py
  __main__.py

Comment thread .envrc Outdated
Comment thread gantry/collection.py Outdated
Comment thread gantry/collection.py Outdated
Comment thread gantry/collection.py Outdated
Comment thread gantry/collection.py Outdated
Comment thread gantry/models/build.py Outdated
Comment thread gantry/models/build.py Outdated
Comment thread gantry/util/gitlab.py Outdated
Comment thread gantry/util/misc.py
Comment thread gantry/views.py Outdated
Co-authored-by: Alec Scott <scott112@llnl.gov>
@cmelone

cmelone commented Jan 24, 2024

Copy link
Copy Markdown
Member Author

I also might suggest creating the sqlite db in the root of the repo and actually putting db/ under gantry/ instead of leaving it as its own top level component.

I like this! Should I stick the .sql schema file in there as well?

@cmelone

cmelone commented Jan 26, 2024

Copy link
Copy Markdown
Member Author

@alecbcs 11d05fc addresses your comments regarding the Prometheus client

lmk it that works

@cmelone

cmelone commented Jan 26, 2024

Copy link
Copy Markdown
Member Author

Going to tackle testing in another PR to avoid adding too much more to this. marking this one as ready

@cmelone
cmelone requested a review from alecbcs January 26, 2024 02:24
@cmelone cmelone mentioned this pull request Feb 5, 2024
2 tasks
alecbcs
alecbcs previously approved these changes Feb 5, 2024

@alecbcs alecbcs 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.

Big fan of the rewrite! Just a few small nitpicks neither of which are real blockers if you prefer keeping as is. Otherwise looks good to me!

Comment thread gantry/clients/prometheus/prometheus.py Outdated
Comment thread gantry/clients/__init__.py Outdated
@alecbcs alecbcs changed the title collection webhook Collection API Feb 5, 2024

@alecbcs alecbcs 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.

Looks good to me. @cmelone thank you for all of your work on this!

@alecbcs
alecbcs merged commit 89353a1 into develop Feb 12, 2024
@alecbcs
alecbcs deleted the add/collection-func branch February 12, 2024 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants