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

Implement config, update search options #21

Merged
merged 22 commits into from
Dec 17, 2024
Merged

Conversation

FlatBallFlyer
Copy link
Contributor

@FlatBallFlyer FlatBallFlyer commented Dec 5, 2024

This refactor implements a seperation of concerns, jest unit testing, and stepci end-to-end testing.

Seperation of concerns

/src/config - Configuration Code
/src/controllers - HTTP request/reply handlers
/src/services - Service layer for RBAC and Business Logic
/src/store - ElastiicIO wrapper
/src/server.ts - Entrypoint to Initilize and Run Server

closes #15
closes #4
closes #13

NOTE: These changes are dependent on code from PR's on the mentorhub and mentorhub-elasticsearch repo's. To test locally you will need to

  • Clone mentorhub, set branch to kafka-cdc and run make install to get docker-compose updates
  • Clone mentorhub-elasticsearch, set branch to version-1.0.0 and run npm run container - new initialize
    You should then be able to run npm run container and npm run stepci to see the API in action.

Dockerfile Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved

// Private Properties
#configFolder: string = "./";
#port: number = 8084;

Choose a reason for hiding this comment

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

Why 8084?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See system port number specifications here: https://github.com/agile-learning-institute/mentorHub/blob/main/specifications/architecture.yaml. Basically all the API's are eventually served from the same host, just at different port numbers.

Copy link

@michquinn michquinn Dec 8, 2024

Choose a reason for hiding this comment

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

I'm still unclear as to why it's 8084 here and 8081 elsewhere in the file. Isn't 8084 for the partner API?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you are correct, will fix this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OH - that is an un-needed initialization, it is over-ridden as the default on line 43 - I chaned this to 0

@FlatBallFlyer FlatBallFlyer merged commit 48ad419 into main Dec 17, 2024
@FlatBallFlyer FlatBallFlyer deleted the implement-config branch December 17, 2024 23:53
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.

Add values to config of the API Add jest unit tests Implement Basic Search API
2 participants