This repository has been archived by the owner. It is now read-only.

Chief transformer refactor #37

Merged
merged 2 commits into from Nov 29, 2012

Conversation

Projects
None yet
2 participants
Contributor

saulius commented Nov 29, 2012

Okay this fix was long due. It is related to the email I sent a while ago. Two major things here:

  1. Certain TARIC updates may invalidate national measures apparently. GoodsNomenclature associated to Measure should span its validity dates (ME8 conformance test). So say we create Measure for GoodsNomenclature that has open ended validity end date. Later Taric updates that GoodsNomenclature to have certain validity date. Some of the national Measures can become invalid. On top of that, CHIEF may try to update those invalid measures later on (just what happened actually). So to prevent it we check for national measure validity after GoodsNomeclature gets updated and do a soft delete (set invalidated by to Taric transaction id) for the measure. Later on, we ignore validations for such measures in case CHIEF will try to update them, so we just allow that. I found that there were 56 national measures invalidated up to 29th of Nov.
  2. Consolidate update application and transformation, so that transformation is invoked after CHIEF updates get applied. This would eventually become an issue for snapshot recreation (in case of need) process. We used to apply all Taric updates, then all CHIEF updates and then do transformation in one go for all CHIEF records. Imagine a scenario where Taric introduces Commodity on 1st of July, CHIEF adds national measures on say the 2nd of July and Taric removes Commodity on 1st of August. If we would apply all Taric's updates then we wouldn't even create CHIEF measure as if that Commodity was never there.
Contributor

saulius commented Nov 29, 2012

I added a snapshot (snapshot_for_29-11-2012.sql.bz2) that has up to date data including invalidated measures.

Contributor

jabley commented Nov 29, 2012

Where possible I would like to add associated tests in smokey or similar. Currently some of our tests are ping-like (is the service there?) rather than checking the content of the page.

Contributor

saulius commented Nov 29, 2012

I'm not sure tests on content are a good thing. Both CHIEF and Taric change that data from time to time so tests may fail even if nothing wrong happened. Do you still want me to list some examples?

jabley added a commit that referenced this pull request Nov 29, 2012

@jabley jabley merged commit 1d8a549 into master Nov 29, 2012

matthewford added a commit that referenced this pull request Jul 11, 2016

Use Amazon S3 in production (#37)
In order to work with a ephemeral filesystem like CloudFoundry we need
to use a external service like Amazon S3

write_file now stores in Amazon S3 when in production

Remove logger methods no longer necessary

Since we are using the local filesystem only in development we no
longer need all this log messages.

Update request stubs in specs

Add logger spec to FileService module

Update specs

fix missing require

Move and update specs

Replace send with conditional

Remove never used error class

Fix infinite loop if the file doesn't exists.

If the file doesn’t exists for some reason, this method will continue
calling itself in a infinite loop.

Remove the TariffSynchronizer rebuild method

This method is no longer important since we will not have the updates
from the filesystem, the sync method should be idempotent

However the main reason is because YAGNI

extract complex method apply from BaseUpdate model to a class

the BaseUpdateImporter will be responsible of calling the import method
of BaseUpdate records

Refactor BaseUpdateImporter

Extract methods from `apply` instance method of the BaseUpdateImporter
class

Add specs for the import method in ChiefUpdate class

Move importer_logger method from TariffImporter

Move importer_logger method from TariffImporter to the ChiefImporter
class where is the only place used

ChiefImporter receives a chief update record instance

Change the ChiefImporter from receiving a path and a non used
issue_date, now receives an instance of chief update. This dependency
injection allows to delegate to the chief update instance to return the
right path

Another change is that the ChiefImporter class no longer inherits from
TariffImporter

Update log messages

Refactor TaricImporter class

Move importer logger specs to importer specs
TaricImporter now expects a taric update object
Fix logger messages
TaricImporter no longer inherits from TariffImporter

Add file_as_stringio instance method to the BaseUpdate class

Remove TariffImporter class

Extract download_content method to TariffUpdatesRequester class

Refactor FileService, is no longer need to be included as a module

Add file_exists? method to FileService class

No need to re-update the taric update record

This record is updated with the filesize when is downloaded, no need to
update this value again

Do not update tariff update record if this already exists

Do not update tariff update with file size if this already exists,
update filesize method

Remove file_as_stringio method from BaseUpdate

Refactor file_path method in BaseUpdate, add specs

Refactor FileService class
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.