Skip to content
This repository has been archived by the owner on Jun 9, 2023. It is now read-only.

Mi fixes #609

Merged
merged 8 commits into from
Oct 12, 2022
Merged

Mi fixes #609

merged 8 commits into from
Oct 12, 2022

Conversation

Gkrumbach07
Copy link
Member

Related Issues and Dependencies

related to: #572

This introduces a breaking change

  • No

This Pull Request implements

Completes:

changes to the source code (e.g. update and version managers), furthermore for specific managers specific files
(e.g. update manager and contributor comparison - how much did bot commit to the Pipfile in opposed to how much did contributor)

Description

Added fail safes for missing env vars. Added a SLI. Added catches to missing data when completing SLI/SLO

@sesheta sesheta added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 20, 2022
xtuchyna
xtuchyna reviewed Sep 23, 2022
View reviewed changes
Copy link
Member

@xtuchyna xtuchyna left a comment

Choose a reason for hiding this comment

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

Good work! Left a few comments and one comment regarding a little discussion, otherwise lgtm :)

srcopsmetrics/cli.py Outdated Show resolved Hide resolved
srcopsmetrics/cli.py Outdated Show resolved Hide resolved
srcopsmetrics/cli.py Outdated Show resolved Hide resolved
srcopsmetrics/kebechet_sli_slo_metrics.py Outdated Show resolved Hide resolved
xtuchyna
xtuchyna suggested changes Sep 30, 2022
View reviewed changes
srcopsmetrics/cli.py Outdated Show resolved Hide resolved
srcopsmetrics/kebechet_sli_slo_metrics.py Outdated Show resolved Hide resolved
srcopsmetrics/kebechet_sli_slo_metrics.py Outdated Show resolved Hide resolved
srcopsmetrics/kebechet_metrics.py Outdated Show resolved Hide resolved
@xtuchyna
Copy link
Member

xtuchyna commented Oct 5, 2022

Hey @Gkrumbach07 , I've suggested some changes, let me know what you think!

@Gkrumbach07
Copy link
Member Author

Hey @Gkrumbach07 , I've suggested some changes, let me know what you think!

Sorry for the delay, I will get to the changes today.

xtuchyna
xtuchyna approved these changes Oct 12, 2022
View reviewed changes
Copy link
Member

@xtuchyna xtuchyna left a comment

Choose a reason for hiding this comment

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

LGTM! thx
/approve

@sesheta
Copy link
Member

sesheta commented Oct 12, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: xtuchyna

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sesheta sesheta added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 12, 2022
@sesheta sesheta merged commit 5b7baea into thoth-station:master Oct 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@xtuchyna xtuchyna xtuchyna approved these changes

@goern goern Awaiting requested review from goern

@mayaCostantini mayaCostantini Awaiting requested review from mayaCostantini

Assignees
No one assigned
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Gkrumbach07 @xtuchyna @sesheta