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: switched from inversify to nestjs #626

Merged
merged 48 commits into from
Apr 3, 2023
Merged

refactor: switched from inversify to nestjs #626

merged 48 commits into from
Apr 3, 2023

Conversation

microwavekonijn
Copy link
Member

@microwavekonijn microwavekonijn commented Sep 22, 2022

Inversify while really feature rich is just a terrible idea for this project. I refactored the code with minimal changes so it uses NestJS, which is inline with other projects like the API and the Kafka collector. We are also able to use the full arsenal of what NestJS has to offer like microservices for instance.

  • Switched to NestJS for IoC;
  • Switched to the NestJS Logger(deprecating Winston);
  • Fixed linting errors.

Some items require attention:

  • Some redundant dependencies: body-parser, cli-color, clone, and cors;
  • Maybe a revisit with regards to the Eslint config as the situation is just nasty(maybe add Prettier);
  • Modularization of the Aggregator with SoC in mind, can be postponed(the majority of the providers are put in the root);
  • Test run the refactor(didn't get around to it);
  • Some configuration changes happened to the logger(relates to deployments).
  • M: Provisioning of the application will have to be re-done, various ENVs may have changed and the k8s health check has definately changed

All in all I found some looming problems while working on this which are addressed now.

Btw, the biggest changes are in the services as I added some modules there. There is also the subscribers from rabbit that have been moved to the root.

@microwavekonijn microwavekonijn added the enhancement New feature or request label Sep 22, 2022
@microwavekonijn microwavekonijn self-assigned this Sep 22, 2022
Copy link
Member

@Maelstromeous Maelstromeous left a comment

Choose a reason for hiding this comment

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

Various comments, mostly related to handling of silly log levels.

Local testing will be required for this, I'm not 100% convinced that the rabbit queues stuff will continue to work with this change, there's a lot of moving parts that, while only IoC in theory has been changed, the effect of singletons (and I'm not seeing now they're declared yet) has drastic effects on the processing.

.eslintrc.js Outdated Show resolved Hide resolved
src/AppModule.ts Show resolved Hide resolved
src/authorities/InstanceAuthority.ts Outdated Show resolved Hide resolved
src/authorities/InstanceAuthority.ts Outdated Show resolved Hide resolved
src/authorities/PopulationAuthority.ts Show resolved Hide resolved
src/middlewares/index.ts Outdated Show resolved Hide resolved
src/services/rabbitmq/queues/AdminQueue.ts Show resolved Hide resolved
src/services/rabbitmq/queues/ApiQueue.ts Show resolved Hide resolved
src/services/rabbitmq/queues/RabbitMQQueue.ts Outdated Show resolved Hide resolved
src/main.ts Show resolved Hide resolved
src/parsers/ZoneDataParser.ts Outdated Show resolved Hide resolved
src/parsers/ZoneDataParser.ts Outdated Show resolved Hide resolved
…attempt to store a local cache of it for multiple lattice versions.

Moved calculation of lattice version to the constants repo for common calculations.
Removed hacking of Oshur data as Census provides it properly now.
Removed local server time calculations, throwback to when we used to factor in server time in brackets (which we no longer do)
…pace them. This has a benefit that Medis can group keys together for ease of administration.

Refactored how census cache hits are recorded.
Removed keys used in one place to describe lists and moved into their respective classes
Removed some extra logging or moved to logger
Corrected duration logic for census cache hits / misses
…named Census cache/miss to be more grammatically correct
Removed k8s templates that no longer used
@Maelstromeous Maelstromeous merged commit 081427c into dev Apr 3, 2023
@Maelstromeous Maelstromeous deleted the feat/nestjs branch April 3, 2023 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants