-
Notifications
You must be signed in to change notification settings - Fork 113
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
Forex Assignment #63
Open
billy-yuan
wants to merge
13
commits into
paidy:master
Choose a base branch
from
billy-yuan:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Forex Assignment #63
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* BY3 convert api response to list * BY3 Add http client * BY3 Make OneFrameApiClient return HashMap * BY3 implement OneFramyDummy.get * BY3 Pull data from OneFrame api * BY3 change one frame api to return Rate * BY3 move one frame into separate directory * BY3 add error as either type in OneFrameApiClient * BY3 delete OneFrameClientApi from Interpreter constructor * BY3 add oneFrameApiClient to constructor * BY4 implement cache (#4) * BY4 install redis * BY4 Implement get in redis * BY4 add scaffiene cache * BY4 save to cache if pair is not in cache * BY4 save to cache in a single transaction * BY4 add RatesCache to constructor * BY6 make scaffiene private * BY3 remove this from rates * BY3 rename OneFrameDummy * BY3 move api token * BY3 put time conversion into Timestamp * BY3 add rename interpreter * BY3 WIP add api response if query param not found * BY3 add invalid pair error as API response * BY3 remove dangling left
* draft of solution * add query parameter section * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md
* add validatedParams class * update api error msg * formatting
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Running the application
port 8000
usingdocker run -p 8000:8080 paidyinc/one-frame
src/main/scala/forex/Main.scala
Example request:
http://localhost:8080/rates?from=USD&to=JPY
Problem Overview
User Story
Constraints
Proposed Solution
Because the number of the requests the Forex service must support is greater than the maximum daily calls allowed by the One-Frame API, we cannot simply forward every request from the service to the One-Frame API.
Instead, we can do the following:
API
Endpoint:
GET /rates
Parameters
from
:string
Starting currencyto
:string
Ending currencyExample request:
GET /rates?from=USD&to=JPY
Responses
Success
Error
Example
error
invalid_rate
: query parameter contains an unsupported rate.interpreter_error
: server error (likely due to a bug)message
Contains details regarding the error.
Http Status Summary
200
OK400
The request was unacceptable, often due to missing a required parameter500
Something went wrong with the interpreter (usually a bug)Minimizing rounding errors
To minimize rounding errors that occurs from taking inverses of currency exchange rates (i.e. if 1 USD = 150.3 yen, the inverse is 1 yen = 1/150.3), we want to include all permutations of rate pairs in the query parameters. In other words:
Why cache and why Caffiene?
The reasons to use a cache are the following:
As for why Caffiene was chosen, it was mostly due to how easy it is to develop locally with it. In addition, the amount of data we need to store is very low (72 pairs of rates) so an in-memory cache is fine.
Calculating cache expiration time
The maximum number of requests we can send to the One-Frame API is as follows:
As long as there is between 1.44 minutes and 5 minutes between every One-Frame API request, we will not hit the daily limit of 1,000 requests. In other words, we can return rates than are at least 1.44 minutes and 5 minutes old.
Now, we can theoretically set the cache expiration time to 5 minutes. However, due to network latency, the returned rate might be a few seconds old by the time the response is received.
Therefore, we will use 3 minutes as the cache expiration.
Edge cases
Rate is not supported by OneFrame API
If a currency in
forex.domain.Currency
is not supported by the One Frame API, then unfortunately the request will fail. However, third party API changes should be prepared for well in advance. If this occurs, I have set up logging so that this issue is easy to detect.Number of supported currencies increases
Currently, there are 9 currencies supported, which means 72 pairs of currencies are sent to the One Frame API. If this increases, then the number of pairs will increase exponentially and the URL may become too long to handle in single request.
If this requirement change happens, then we will have to*
Limitations
Caffiene is in-memory and cannot be used by multiple instances
If more than one instance of this service is spun up, then each instance will have its own separate cache. Because the caches do not share data, they each need to fetch data using the OneFrame API client once its data expires. This will increase the number of API requests.
To prevent both issues, a centralized cache can be used (like Redis) by all instances.
Historical rates cannot be queried
Because rates are stored in a cache and expire after 3 minutes, old rates cannot be queried. If this becomes a requirement in the future, then we can add this feature and save the rates to a database in addition to storing the data to a cache.
Improvements & final thoughts
match
blocks that handle monads (as this is my first time using Scala, the code I have written is not the most idiomatic)from
andto
query parameters are the same)