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

Kma 163213435 write import fuel function #1793

Merged
merged 50 commits into from Mar 5, 2019
Merged
Changes from 32 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
a648bc5
Add pseudocode for function to save fuel price data
kimallen Feb 6, 2019
0d4fc4b
Add function to calculate baseline rate; add testing file; fill out s…
kimallen Feb 7, 2019
6bbb49e
Write function to fetch twelve months of fuel prices from db
kimallen Feb 7, 2019
aa1dbcb
Add test for fetch of last 12 months of fuel data
kimallen Feb 8, 2019
b31c73f
Debug off-by-1 error in test
kimallen Feb 8, 2019
788438b
Move files to service directory; fill out local functions; refactor test
kimallen Feb 9, 2019
b9f1e6a
Flush out fields and values to save in DB; add functionality to retur…
kimallen Feb 11, 2019
05737b0
Add a CLI command that calls the fuel price function
kimallen Feb 12, 2019
0e5e518
Adjust datestring format and loop to get correct months from db; add …
kimallen Feb 12, 2019
966b03b
Add more error handling; adjust structs to be able to access fuel dat…
kimallen Feb 13, 2019
b396c69
Add error handling; uncomment and adjust code to get earliest week of…
kimallen Feb 13, 2019
e9925b8
Add components for adding EIA_KEY as an envt variable
kimallen Feb 13, 2019
e5c69b0
Move files from service directory to services directory (and remove s…
kimallen Feb 14, 2019
756d981
Add (failing) test for AddFuelDieselPricesCall
kimallen Feb 14, 2019
89edd43
Change names to align with our service object conventions; fix format…
kimallen Feb 16, 2019
f558c26
Rename files again to match struct rather than method
kimallen Feb 18, 2019
c425bfa
Replace references to time.Now() with clock; Make fetch of last few m…
kimallen Feb 20, 2019
9acf402
Account for error returned as successful json response; add error han…
kimallen Feb 21, 2019
9045142
Fix key name in flag; add to config definitions
kimallen Feb 21, 2019
82ee2eb
Fix query using date_part; adjust years for currentDate and shipmentD…
kimallen Feb 21, 2019
75502cc
Pull out actual fetch into its own function
kimallen Feb 21, 2019
5834f55
Add fetch funciton as part of struct and start mock function for test…
kimallen Feb 22, 2019
10c7c2d
Set up api key and eia url as environment variablesto; fix 1 off bug …
kimallen Feb 23, 2019
4f6de55
Use function to hide url and key; use raw url for container configs
kimallen Feb 25, 2019
a678e4a
Parse and add query to url
kimallen Feb 25, 2019
394bdb2
Make log more explicit
kimallen Feb 26, 2019
f234c41
Add function to find the first Monday, or next non-holiday after the …
kimallen Feb 27, 2019
31f2b03
Outline tests to write in test file
kimallen Feb 27, 2019
2327ff8
Fill in test cases and test case data; remove commented code
kimallen Feb 28, 2019
16dcc73
Fix data structure in mocked responses; move tests to where db is in …
kimallen Feb 28, 2019
963afa9
Push changes on Gopkg from new pkg
kimallen Feb 28, 2019
dfa4e03
Add test for BaselineRate value saved to db
kimallen Feb 28, 2019
3e4d8f7
Fix type of expected values and change to use actual values from onli…
kimallen Feb 28, 2019
e385f5d
Make baselineRate 0 if the fuel price is less than or equal to the ba…
kimallen Feb 28, 2019
f767abc
Fix indenting and be more specific in error message
kimallen Mar 1, 2019
6c46709
Fix verrs error conditional; refactor getFirstMondayOrHolidayAfter; r…
kimallen Mar 1, 2019
1cc84ff
Add missing server test; adjust awkward error message
kimallen Mar 1, 2019
6804ae0
Make clocks UTC; rewrite query style for BETWEEN
kimallen Mar 1, 2019
5a07dbe
Merge branch 'master' into kma-163213435-write-import-fuel-function
kimallen Mar 2, 2019
5c3142a
Account for Monday holiday; refactor conditionals
kimallen Mar 2, 2019
afb3626
Change type of OtherData to accomodate unknown data; add test for non…
kimallen Mar 3, 2019
50c3013
Fix path to cmd file
kimallen Mar 4, 2019
45d9f36
Add omitempty to fields that could not exist in data structure
kimallen Mar 4, 2019
ac45160
Use more specific format types in string interpolation; remove TODO c…
kimallen Mar 4, 2019
770ca2d
Replace default logger with custom logger
kimallen Mar 4, 2019
99e035e
Refactor pubDateString conversion; fix zap.logger type conflict
kimallen Mar 4, 2019
1cdf848
Move tests into subtests; add comments to explain formula used to get…
kimallen Mar 5, 2019
7babf17
Add comment for intInSlice function
kimallen Mar 5, 2019
b2a1b9b
Handle as error if type assertions fail by using 'ok' syntax
kimallen Mar 5, 2019
ab10e52
Merge branch 'master' into kma-163213435-write-import-fuel-function
kimallen Mar 5, 2019
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.
+747 −7
Diff settings

Always

Just for now

4 .envrc
@@ -155,6 +155,10 @@ export IWS_RBS_HOST="pkict.dmdc.osd.mil"
# Unsecured CSRF Auth Key, for local dev only
require CSRF_AUTH_KEY "See https://docs.google.com/document/d/1DuWXZLFaW7FXvqh-PStqjZI40niEavXWS5PPtWPlK3w"

# EIA API Key (for fuel price data)
require EIA_KEY "https://docs.google.com/document/d/1K1-xlYcZaS518PQiaB39gSvqz2tTo0W8eM0wImB7TcI"
export EIA_URL="https://api.eia.gov/series/"

##############################################
# Load Local Overrides and Check Environment #
##############################################

Some generated files are not rendered by default. Learn more.

Oops, something went wrong.
@@ -0,0 +1,47 @@
package main

import (
"log"
"os"
"strings"

"github.com/facebookgo/clock"
"github.com/gobuffalo/pop"
"github.com/spf13/pflag"
"github.com/spf13/viper"

"github.com/transcom/mymove/pkg/services/fuelprice"
)

// Command: go run cmd/save_fuel_price_data/main.go
func main() {

flag := pflag.CommandLine

flag.String("eia-key", "", "key for Energy Information Administration (EIA) api")
flag.String("eia-url", "", "url for EIA api")
flag.Parse(os.Args[1:])

v := viper.New()
v.BindPFlags(flag)
v.SetEnvKeyReplacer(strings.NewReplacer("-", "_"))
v.AutomaticEnv()

db, err := pop.Connect("development")
if err != nil {
log.Fatal(err)
}
clock := clock.New()
fuelPrices := fuelprice.NewDieselFuelPriceStorer(
db,
clock,
fuelprice.FetchFuelPriceData,
v.GetString("eia-key"),
v.GetString("eia-url"),
)

verrs, err := fuelPrices.StoreFuelPrices(12)
if err != nil || verrs != nil {
log.Fatal(err, verrs)
This conversation was marked as resolved by chrisgilmerproj

This comment has been minimized.

Copy link
@chrisgilmerproj

chrisgilmerproj Feb 28, 2019

Contributor

When I ran the script I got this output:

2019/02/28 22:54:49 Fuel Data added for February with pub_date 2019-02-04 00:00:00 +0000 UTC
2019/02/28 22:54:49 <nil>
exit status 1

Which makes me think that something is going on with this log.Fatal statement. Can you check to make sure this is being done right? I don't expect to get an exit code of 1 or a log line like this.

This comment has been minimized.

Copy link
@reggieriser

reggieriser Mar 1, 2019

Contributor

I think the problem is that you're checking verrs != nil instead of verrs.HasAny(). The verrs is an empty struct (i.e., non-nil) even when you have no validation errors.

This comment has been minimized.

Copy link
@kimallen

kimallen Mar 1, 2019

Author Contributor

fixed

}
}
@@ -265,6 +265,10 @@ func initFlags(flag *pflag.FlagSet) {

// CSRF Protection
flag.String("csrf-auth-key", "", "CSRF Auth Key, 32 byte long")

// EIA Open Data API
flag.String("eia-key", "", "Key for Energy Information Administration (EIA) api")
flag.String("eia-url", "", "Url for Energy Information Administration (EIA) api")
}

func initDODCertificates(v *viper.Viper, logger *webserverLogger) ([]tls.Certificate, *x509.CertPool, error) {
@@ -518,6 +522,16 @@ func checkConfig(v *viper.Viper) error {
return err
}

err = checkEIAKey(v)
if err != nil {
return err
}

err = checkEIAURL(v)
if err != nil {
return err
}

return nil
}

@@ -642,6 +656,22 @@ func checkGEX(v *viper.Viper) error {
return nil
}

func checkEIAKey(v *viper.Viper) error {
eiaKey := v.GetString("eia-key")
if len(eiaKey) != 32 {
return fmt.Errorf("eia key is not 32 characters key is %d chars", len(eiaKey))

This comment has been minimized.

Copy link
@reggieriser

reggieriser Mar 1, 2019

Contributor

Nit: Error message reads awkwardly -- needs punctuation or rewording.

This comment has been minimized.

Copy link
@kimallen

kimallen Mar 2, 2019

Author Contributor

adjusted

}
return nil
}

func checkEIAURL(v *viper.Viper) error {
eiaURL := v.GetString("eia-url")
if eiaURL != "https://api.eia.gov/series/" {
This conversation was marked as resolved by chrisgilmerproj

This comment has been minimized.

Copy link
@chrisgilmerproj

chrisgilmerproj Feb 28, 2019

Contributor

You may also want to do what we do in the gexURL, if len(eiaURL) > 0 && ...

This comment has been minimized.

Copy link
@chrisgilmerproj

chrisgilmerproj Feb 28, 2019

Contributor

also, the gex url error is a bit more clear about what is expected. I'd suggest copying that here.

This comment has been minimized.

Copy link
@kimallen

kimallen Mar 1, 2019

Author Contributor

I'm adding a better message about what is expected. The len thing for gexURL was specifically because the url could be empty or the exact url. eia-url can only be the exact url.

return fmt.Errorf("eia url not correct as %s", eiaURL)
}
return nil
}

func checkStorage(v *viper.Viper) error {

storageBackend := v.GetString("storage-backend")
@@ -110,6 +110,10 @@ func (suite *webServerSuite) TestConfigGEX() {
suite.Nil(checkGEX(suite.viper))
}

func (suite *webServerSuite) TestConfigEIAKey() {
This conversation was marked as resolved by chrisgilmerproj

This comment has been minimized.

Copy link
@reggieriser

reggieriser Mar 1, 2019

Contributor

Do we also need a TestConfigEIAURL()?

This comment has been minimized.

Copy link
@kimallen

kimallen Mar 2, 2019

Author Contributor

added

suite.Nil(checkEIAKey(suite.viper))
}

func (suite *webServerSuite) TestConfigStorage() {
suite.Nil(checkStorage(suite.viper))
}
@@ -138,10 +138,18 @@
"name": "GEX_URL",
"value": "https://gexweba.daas.dla.mil/msg_data/submit/"
},
{
"name": "SEND_PROD_INVOICE",
"value": "{{ .SEND_PROD_INVOICE }}"
}
{
"name": "SEND_PROD_INVOICE",
"value": "{{ .SEND_PROD_INVOICE }}"
},
{
"name": "EIA_KEY",
"value": "{{ .EIA_KEY }}"
},
{
"name": "EIA_URL",
"value": "https://api.eia.gov/series/"
}

This comment has been minimized.

Copy link
@chrisgilmerproj

chrisgilmerproj Feb 28, 2019

Contributor

This is a nitpick, but can you fix the indentation here?

This comment has been minimized.

Copy link
@kimallen

kimallen Feb 28, 2019

Author Contributor

I keep trying, but it won't let me!

This comment has been minimized.

Copy link
@kimallen

kimallen Feb 28, 2019

Author Contributor

Ok, got it- i had to open the repo in Sublime instead of Goland since it was forcing the weird indenting.

This comment has been minimized.

Copy link
@reggieriser

reggieriser Feb 28, 2019

Contributor

FYI, in GoLand, you can do a shift-tab on the line you want to unindent. The code style for JSON in your GoLand project probably has a tab size of 4 that it's trying to enforce. You could change that to 2 if you want, assuming other JSON files you may need to edit have 2-space indents.

],
"logConfiguration": {
"logDriver": "awslogs",
@@ -161,6 +161,14 @@
{
"name": "SEND_PROD_INVOICE",
"value": "{{ .SEND_PROD_INVOICE }}"
},
{
"name": "EIA_KEY",
"value": "{{ .EIA_KEY }}"
},
{
"name": "EIA_URL",
"value": "https://api.eia.gov/series/"
}
],
"logConfiguration": {
@@ -6,4 +6,4 @@ HTTP_SDDC_SERVER_NAME=mymove-experimental.sddc.army.mil
DPS_REDIRECT_URL=https://dpstest.sddc.army.mil/cust
DPS_COOKIE_NAME=DPSIVV
GEX_URL=https://gexweba.daas.dla.mil/msg_data/submit/
SEND_PROD_INVOICE=false
SEND_PROD_INVOICE=false
@@ -4,10 +4,12 @@ import (
"encoding/json"
"time"

"github.com/facebookgo/clock"
"github.com/gobuffalo/pop"
"github.com/gobuffalo/validate"
"github.com/gobuffalo/validate/validators"
"github.com/gofrs/uuid"
"github.com/pkg/errors"

"github.com/transcom/mymove/pkg/unit"
)
@@ -41,6 +43,20 @@ func (f FuelEIADieselPrices) String() string {
return string(jf)
}

// FetchMostRecentFuelPrices queries and fetches all fuel_eia_diesel_prices for past specified number of months
func FetchMostRecentFuelPrices(dbConnection *pop.Connection, clock clock.Clock, numMonths int) ([]FuelEIADieselPrice, error) {
today := clock.Now()
query := dbConnection.Where("pub_date >= $1 AND pub_date <= $2", today.AddDate(0, -numMonths, 0), today)

This comment has been minimized.

Copy link
@reggieriser

reggieriser Mar 1, 2019

Contributor

Does timezone factor in here? I believe clock.Now() is going to give you local time, not UTC.

Just an FYI ... I think you can also use something like pub_date BETWEEN $1 AND $2 which may read a little better (but that's really just a style choice).

This comment has been minimized.

Copy link
@kimallen

kimallen Mar 2, 2019

Author Contributor

added UTC to this and the test clocks. I do wonder if we should factor what time zone the fuel data is updated in the api? Probably doesnt really matter- if the function is run daily, we know that if it's not retrieved the day it's posted, it would be retrieved the next day.

This comment has been minimized.

Copy link
@reggieriser

reggieriser Mar 4, 2019

Contributor

Agreed -- it probably won't ultimately matter in this case.

// TODO: what if today's date is before the first Monday/publication date, but in the new month?

This comment has been minimized.

Copy link
@chrisgilmerproj

chrisgilmerproj Feb 28, 2019

Contributor

I feel like you need to catch this somewhere in your handler code? Not sure what else you could do here.

This comment has been minimized.

Copy link
@kimallen

kimallen Mar 1, 2019

Author Contributor

After examining this, I think it's okay to include a check for data for this month as part of the numMonths to check-- it's more a matter of how I want to define "Most recent fuel prices". The usage of this function isn't impacted by whether I get numMonths back or numMonths + 1 back of data. It's still going to replace the missing month and cover the desired number of months.

var fuelPrices []FuelEIADieselPrice

This comment has been minimized.

Copy link
@reggieriser

reggieriser Mar 1, 2019

Contributor

There's already an FuelEIADieselPrices type for this if you want to use it.

err := query.Eager().All(&fuelPrices)

if err != nil {
return fuelPrices, errors.Wrap(err, "Fetch line items query failed")
}
return fuelPrices, nil
}

// Validate gets run every time you call a "pop.Validate*" (pop.ValidateAndSave, pop.ValidateAndCreate, pop.ValidateAndUpdate) method.
// This method is not required and may be deleted.
func (f *FuelEIADieselPrice) Validate(tx *pop.Connection) (*validate.Errors, error) {
@@ -4,6 +4,7 @@ import (
"testing"
"time"

"github.com/facebookgo/clock"
"github.com/gofrs/uuid"

"github.com/transcom/mymove/pkg/models"
@@ -146,6 +147,31 @@ func (suite *ModelSuite) TestMakeFuelEIADieselPriceForDate() {
testdatagen.MakeFuelEIADieselPriceForDate(suite.DB(), shipmentDate, assertions)
}

func (suite *ModelSuite) TestFetchMostRecentFuelPrices() {
// Make fuel price records for the last twelve months
clock := clock.NewMock()
currentTime := clock.Now()
for month := 0; month < 15; month++ {
var shipmentDate time.Time

shipmentDate = currentTime.AddDate(0, -(month - 1), 0)
testdatagen.MakeDefaultFuelEIADieselPriceForDate(suite.DB(), shipmentDate)
}

fuelPrices, err := models.FetchMostRecentFuelPrices(suite.DB(), clock, 12)
expectedNumFuelPrices := 12
suite.NoError(err)
suite.Equal(expectedNumFuelPrices, len(fuelPrices))

// if the day is after the 15th
currentTime = currentTime.Add(time.Hour * 24 * 20)
fuelPrices, err = models.FetchMostRecentFuelPrices(suite.DB(), clock, 12)
expectedNumFuelPrices = 12
suite.NoError(err)
suite.Equal(expectedNumFuelPrices, len(fuelPrices))
// TODO: another test to make sure earliest and latest month match?

This comment has been minimized.

Copy link
@chrisgilmerproj

chrisgilmerproj Mar 4, 2019

Contributor

nit: Add the test or remove the TODO :)

This comment has been minimized.

Copy link
@kimallen

kimallen Mar 4, 2019

Author Contributor

removed- I don't even understand what I meant there

}

// Create 1 record for the shipment date provided
func (suite *ModelSuite) TestMakeDefaultFuelEIADieselPriceForDate() {
shipmentDate := time.Now()
Oops, something went wrong.
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.