-
Notifications
You must be signed in to change notification settings - Fork 97
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
[Feature Contribution]: Remove JSON examples from WebAPI container #440
Comments
As a user who's experimenting with implementing POC instances of Carbon Aware SDK I feel the sample data should be there to confirm first-instance usage (i.e. I want to focus on deploying and ensuring I can get a result, once I've done that I can change the data source to be WattTime/Electricity Maps). So I personally wouldn't want the sample data to be removed. |
AFAIK RDBMS container (e.g. PostgreSQL) does not contain sample data. So it is nature not to contain them in WebAPI container IMHO. If you want to confirm your deployment status, you can use healthcheck endpoint ( I agree that sample data is useful to try to the SDK, so they should keep in the repo. I just want to say they should be removed from WebAPI container. |
@YaSuenag fair enough! Thanks for the clarification. |
Further investigation, I think it is better to do following things rather than just remove JSON files from WebAPI container. I will send PR if we can make agreement. About appsettings.json, we need to remove following lines: carbon-aware-sdk/src/CarbonAware.WebApi/src/appsettings.json Lines 2 to 11 in d8247a2
In addirtion, it might be better to remove both location JSON and emission data JSON from packaging item: carbon-aware-sdk/src/GSF.CarbonAware/src/GSF.CarbonAware.csproj Lines 52 to 53 in d8247a2
However some files depend on test-data-azure-emissions.json - I think we should remove these dependencies. Line 12 in d8247a2
carbon-aware-sdk/src/CarbonAware.WebApi/src/carbon-aware.json Lines 14 to 16 in d8247a2
|
Closes Green-Software-Foundation#440 Signed-off-by: Yasumasa Suenaga <[email protected]>
This issue has not had any activity in 120 days. Please review this issue and ensure it is still relevant. If no more activity is detected on this issue for the next 20 days, it will be closed automatically. |
Wait! I've sent PR for this issue: #450 |
This issue has not had any activity in 120 days. Please review this issue and ensure it is still relevant. If no more activity is detected on this issue for the next 20 days, it will be closed automatically. |
This issue does not stale! I've sent PR #450 for this issue, and I will fix conflictions when other PRs are merged. |
What happened?
I'd like to propose to remove following JSON files from WebAPI container:
They are example files to try WebAPI easily, I think. But they are not necessary in the container use case because container user should set their account for backend services (e.g. Electricity Maps, WattTime) and location config. If they exist in container, the user might not be aware soon their wrong when some configurations are missed.
Code of Conduct
Feature Commitment
The text was updated successfully, but these errors were encountered: