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

Refactor mapping service #50

Merged
merged 4 commits into from
Mar 14, 2025
Merged

Refactor mapping service #50

merged 4 commits into from
Mar 14, 2025

Conversation

tkan145
Copy link
Contributor

@tkan145 tkan145 commented Mar 13, 2025

Small refactor, including:

  • Only call rover install when running busted test to avoid overwriting dependencies in the image with the image from Roverfile.lock
  • Refactor busted file to use production image instead of s2i

The only downside to this refactoring is that it will download the luarocks packages every time you run the busted test. But given the frequency of updates in this repository, I think we can live with that.

tkan145 added 4 commits March 10, 2025 11:07
lua-resty-iputils is already included in the base image
 We are using the production image which should contains all required
 packages, hence reinstall packages with rover is unneeded.
@tkan145 tkan145 force-pushed the refactor-mapping-service branch from 3725cdb to 69e3969 Compare March 13, 2025 11:47
@tkan145 tkan145 requested review from slopezz and roivaz March 14, 2025 07:28
Copy link
Contributor

@roivaz roivaz left a comment

Choose a reason for hiding this comment

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

Thanks for tidying up the house! Could you remove the wip from the PR title?

Copy link
Member

@slopezz slopezz left a comment

Choose a reason for hiding this comment

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

Thank you very much for the PR @tkan145 !!

Does the PR solves the issue detected in staging mapping-service (bad argument #2 to 'connect' (string expected, got table)?

If so, please remove the WIP from PR title, we will merge the PR and I will create a new image to test it in staging

@tkan145 tkan145 changed the title [WIP] Refactor mapping service Refactor mapping service Mar 14, 2025
@tkan145
Copy link
Contributor Author

tkan145 commented Mar 14, 2025

@slopezz it should resolve the bad argument #2 to 'connect' (string expected, got table) error 🤞

Removed WIP

@slopezz slopezz merged commit e64051f into master Mar 14, 2025
2 checks passed
@slopezz slopezz deleted the refactor-mapping-service branch March 14, 2025 10:31
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