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

test handle service 2 container endpoints #122

Merged
merged 6 commits into from
Jul 16, 2024

Conversation

Xiangs18
Copy link
Collaborator

No description provided.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Lintly has detected code quality issues in this pull request.

@Xiangs18 Xiangs18 requested review from MrCreosote and removed request for github-actions[bot] July 15, 2024 23:06
Copy link

codecov bot commented Jul 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.13%. Comparing base (17a8210) to head (d93f886).

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #122   +/-   ##
========================================
  Coverage    84.13%   84.13%           
========================================
  Files            5        5           
  Lines          416      416           
========================================
  Hits           350      350           
  Misses          66       66           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

.github/workflows/test.yaml Outdated Show resolved Hide resolved
container_test/handle_service_container_test.py Outdated Show resolved Hide resolved
FILE_NAME = "mylittlefile"

HS_URL = "http://localhost:8080"
BLOB_URL = "https://ci.kbase.us/services/shock-api"
Copy link
Member

@MrCreosote MrCreosote Jul 15, 2024

Choose a reason for hiding this comment

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

If you want to make this truly standalone you could add auth and the blobstore to the docker compose, similar to how the workspace does it with auth. Then you don't need a token. Maybe not enough ROI though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think this is good for now. If you think is necessary, I can address this after I get myself unblocked from jgi-kbase/IDMappingService#140

Copy link
Member

Choose a reason for hiding this comment

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

I'm ambivalent

container_test/handle_service_container_test.py Outdated Show resolved Hide resolved
container_test/handle_service_container_test.py Outdated Show resolved Hide resolved
container_test/handle_service_container_test.py Outdated Show resolved Hide resolved
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Lintly has detected code quality issues in this pull request.

@Xiangs18 Xiangs18 requested a review from MrCreosote July 16, 2024 15:54
Copy link
Member

@MrCreosote MrCreosote left a comment

Choose a reason for hiding this comment

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

LGTM. One minor comment that isn't a big deal

max_attempts = len(WAIT_TIMES) + 1
while attempt <= max_attempts:
print(f"Attempt {attempt} of {max_attempts}")
hs = AbstractHandle(HS_URL)
Copy link
Member

Choose a reason for hiding this comment

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

You could move this out of the loop, no need to reconstruct it every time

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

👍

@Xiangs18 Xiangs18 dismissed github-actions[bot]’s stale review July 16, 2024 18:47

Lintly doesn't seem to dismiss old reviews when it passes

@Xiangs18 Xiangs18 merged commit 069e04e into develop Jul 16, 2024
14 checks passed
@Xiangs18 Xiangs18 deleted the dev-test_container_service branch July 16, 2024 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants