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

chore(job_attachments): remove service model file #193

Merged
merged 1 commit into from
Mar 4, 2024

Conversation

gahyusuh
Copy link
Contributor

@gahyusuh gahyusuh commented Feb 29, 2024

What was the problem/requirement? (What/Why)

This package previously had a copy of the service model file (JSON) for use in these unit tests and by GitHub Checks to access the boto3 deadline client's stubber fixture. The service model file was at data/boto_module/deadline/1989-09-22/service.json. This setup seemed unnecessary for several reasons:

  • The get_queue() in (_aws/deadline.py) has already been covered multiple times in integration tests, so it seemed unnecessary to test it in unit tests by patching the service model file directly.
  • We don't have a reliable mechanism to keep this copy of service model file up-to-date, especially considering it hadn't been updated in the past four months.

What was the solution? (How)

  • Removed the service model file from the package.
  • Refactored the unit tests that depended on this file, implementing mocks to ensure they still pass without needing to access the service model file.

What is the impact of this change?

Remove unnecessary dependencies and potential points of failure due to out-of-date service model.

How was this change tested?

Confirmed that all unit tests and integration tests (for job attachments) were passing.

Was this change documented?

No.

Is this a breaking change?

No.

@gahyusuh gahyusuh force-pushed the gahyusuh/remove_model_file branch 6 times, most recently from c931e41 to c949fac Compare March 3, 2024 16:03
@gahyusuh gahyusuh force-pushed the gahyusuh/remove_model_file branch from c949fac to ad2ac37 Compare March 3, 2024 16:21
@gahyusuh gahyusuh marked this pull request as ready for review March 3, 2024 16:33
@gahyusuh gahyusuh requested a review from a team as a code owner March 3, 2024 16:33
@gahyusuh gahyusuh merged commit 6c9142e into mainline Mar 4, 2024
18 checks passed
@gahyusuh gahyusuh deleted the gahyusuh/remove_model_file branch March 4, 2024 16:02
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.

3 participants