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.

Sign up for GitHub

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

Jump to bottom

[tests] Add conversion of prometheus docker integration test - part 1 #3921

Merged
merged 3 commits into from
Nov 21, 2021
Merged

[tests] Add conversion of prometheus docker integration test - part 1 #3921

merged 3 commits into from
Nov 21, 2021

Conversation

nbroyles
Copy link
Collaborator

What this PR does / why we need it:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing and/or backwards incompatible change?:

Does this PR require updating code package or user-facing documentation?:

@nbroyles nbroyles requested a review from fishie9 November 15, 2021 17:10
@codecov
Copy link

codecov bot commented Nov 15, 2021 •
edited
Loading

Codecov Report

Merging #3921 (5051d07) into master (5051d07) will not change coverage.
The diff coverage is n/a.

❗ Current head 5051d07 differs from pull request most recent head 65a2017. Consider uploading reports for the commit 65a2017 to get more accurate results

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #3921   +/-   ##
======================================
  Coverage    56.9%   56.9%           
======================================
  Files         555     555           
  Lines       63386   63386           
======================================
  Hits        36071   36071           
  Misses      24129   24129           
  Partials     3186    3186
Flag Coverage Δ
aggregator 62.4% <0.0%> (ø)
cluster ∅ <0.0%> (∅)
collector 58.4% <0.0%> (ø)
dbnode 60.7% <0.0%> (ø)
m3em 46.4% <0.0%> (ø)
metrics 19.7% <0.0%> (ø)
msg 74.1% <0.0%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5051d07...65a2017. Read the comment docs.

Base automatically changed from nb/coord-apis to master November 15, 2021 23:17
@nbroyles nbroyles force-pushed the nb/convert-prom branch 2 times, most recently from 6e2ef30 to f6c28c2 Compare November 15, 2021 23:33
wesleyk
wesleyk reviewed Nov 19, 2021
View reviewed changes
src/integration/resources/resources.go
@@ -89,18 +89,46 @@ func SetupCluster(
}

var (
aggDatabase = admin.DatabaseCreateRequest{
unaggDatabase = admin.DatabaseCreateRequest{
Copy link
Collaborator

Choose a reason for hiding this comment

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

ah so now by default test clusters will have two namespaces?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, it already does. They were just being created incorrectly here previously. This also mirrors what we do with the existing integration tests.

wesleyk
wesleyk reviewed Nov 19, 2021
View reviewed changes
src/integration/resources/resources.go
Enabled: true,
BlockSizeNanos: int64(30 * time.Minute),
},
RetentionOptions: &namespace.RetentionOptions{
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe this should be defined as a default constant?

@nbroyles nbroyles merged commit 9dcae30 into master Nov 21, 2021
@nbroyles nbroyles deleted the nb/convert-prom branch November 21, 2021 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wesleyk wesleyk wesleyk approved these changes

@fishie9 fishie9 Awaiting requested review from fishie9

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
@nbroyles @wesleyk