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

add local file storage target #6

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hirosassa
Copy link

@hirosassa hirosassa commented Jun 13, 2021

What is this PR?

This PR adds storage target LocalFile of local file instead of Firestore.
When you use LocalFile, descriptions fetched from BigQuery will be stored in local directory as json files.
In my use case, I need simpler solution than Firestore for some reasons.
I want to discuss with reviewers whether this feature is preferable for this repo or not.

Thanks

@@ -8,6 +8,10 @@ class Config(object):
# valid choice : "info", "warn", "error", "debug"
loglevel = "info"

# Storage Target
# valid choice : "localfile", "firestore"
Copy link
Author

Choose a reason for hiding this comment

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

[need discussions] I'll add gcs target and/or the other object storages in another PR. How do you think about this idea?

from lib.table_desc import TableDesc


class Storage(metaclass=ABCMeta):
Copy link
Author

Choose a reason for hiding this comment

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

for extensibility of storage target, I implemented base class for storage

@@ -1,3 +1,6 @@
#! /bin/bash
set -eu
Copy link
Author

Choose a reason for hiding this comment

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

for safety

from lib.table_desc import TableDesc

TEST_DS = "test_bqdesc_buckuper"
TEST_DS = "test_bqdesc_backuper"
Copy link
Author

Choose a reason for hiding this comment

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

just fix typo

class TestControllerBase(unittest.TestCase):
"""Base class for controller test case. `self.storage` will be parameterized.
"""
__test__ = False # HACK: skip base class tests
Copy link
Author

@hirosassa hirosassa Jun 13, 2021

Choose a reason for hiding this comment

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

To prevent running base class tests.
But this hack makes that we cannot use python -m unittest discover test because unittest discover command does not respect __test__ variables.
I'll add tox configurations to solve this problem later.


def setUp(self):
super().setUp()
self.storage = Firestore(config=config, logger=logger)
Copy link
Author

Choose a reason for hiding this comment

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

inject storage variable

@hirosassa hirosassa changed the title WIP: add local file storage target add local file storage target Jun 13, 2021
@hirosassa
Copy link
Author

@TetsutaroWatanabe Plz review!

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.

1 participant