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

Freature/atlas serializable #1

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Conversation

dechoma
Copy link
Owner

@dechoma dechoma commented May 17, 2021

Summary of Changes

Tests

Documentation

CheckList

Make sure you have checked all steps below to ensure a timely review.

  • PR title addresses the issue accurately and concisely. Example: "Updates the version of Flask to v1.0.2"
  • PR includes a summary of changes.
  • PR adds unit tests, updates existing unit tests, OR documents why no test additions or modifications are needed.
  • In case of new functionality, my PR adds documentation that describes how to use it.
    • All the public functions and the classes in the PR contain docstrings that explain what it does


class FsAtlasCSVLoader(Loader):
"""
Write node and relationship CSV file(s) that can be consumed by
Copy link

Choose a reason for hiding this comment

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

Write entity?

"""
Write node and relationship CSV file(s) that can be consumed by
AtlasCsvPublisher.
It assumes that the record it consumes is instance of AtlasCsvSerializable
Copy link

Choose a reason for hiding this comment

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

shouldn't it just be AtlasSerializable?




def _get_neo4j_suffix_value(value: Any) -> str:
Copy link

Choose a reason for hiding this comment

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

do we need this?

# Directory should be deleted after publish is finished
Job.closer.register(_delete_dir)

def load(self, csv_serializable: AtlasSerializable) -> None:
Copy link

Choose a reason for hiding this comment

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

should it just be serializable?


from collections import namedtuple

AtlasEntity = namedtuple(
Copy link

Choose a reason for hiding this comment

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

is it possible to enforce presence of qualifiedName within attributes?

super(AtlasCSVPublisher, self).__init__()

def init(self, conf: ConfigTree) -> None:
self._atlas_client = AtlasClient('http://localhost:21000', ('admin', 'admin'))
Copy link

Choose a reason for hiding this comment

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

this one can be pretty much copied from atlas_search_data_extractor.py

TYPE_NAME: entity.typeName
}
for key, value in entity.attributes.items():
key_suffix = '-elo'
Copy link

Choose a reason for hiding this comment

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

why is it needed?

from databuilder.models.atlas_entity import AtlasEntity
from databuilder.models.atlas_relationship import AtlasRelationship

GUID = 'guid'
Copy link

Choose a reason for hiding this comment

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

idea: amundsen-io#1103 introduces atlas_utils.py in amundsen_common which could be reused here

dechoma and others added 3 commits May 19, 2021 16:04
Signed-off-by: DOMINIK CHOMA <dominik.choma@ing.com>
Signed-off-by: DOMINIK CHOMA <dominik.choma@ing.com>
Signed-off-by: DOMINIK CHOMA <dominik.choma@ing.com>
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.

2 participants