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

Create New Connections per #68 #69

Merged
merged 5 commits into from
Aug 15, 2016
Merged

Create New Connections per #68 #69

merged 5 commits into from
Aug 15, 2016

Conversation

t8y8
Copy link
Contributor

@t8y8 t8y8 commented Jul 30, 2016

This is all that we'd need for creating connections from scratch.

Authentication isn't validated right now so it's not very safe, it defaults to '' because I need to research more how this attribute works

Feedback:

  • Name. The issue proposes .create() but our current pattern is from_something
  • Approach -- though it's simple
  • Thoughts on authentication
  • Note I fixed a bug I introduced, the name of the dbclass in the xml is actually 'class'

@@ -120,4 +133,4 @@ def dbclass(self, value):
raise AttributeError("'{}' is not a valid database type".format(value))

self._class = value
self._connectionXML.set('dbclass', value)
self._connectionXML.set('class', value)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Totally a bug in my dbclass code.

@t8y8
Copy link
Contributor Author

t8y8 commented Jul 30, 2016

Alright, I've totally got a prototype working for creating datasources (V10) as well. It's ugly but wanted to get it out for discussion.

    @classmethod
    def from_scratch(cls, caption, connections):
        from uuid import uuid4
        unique = lambda x: x + '.' + str(uuid4()).replace('-', '')[:28]
        root = ET.Element('datasource')
        root.set('caption', caption)
        root.set('version', '10.0')
        outer_connection = ET.SubElement(root, 'connection')
        outer_connection.set('class', 'federated')
        named_conns = ET.SubElement(outer_connection, 'named-connections')
        for conn in connections:
            nc = ET.SubElement(named_conns, 'named-connection')
            nc.set('name', unique(conn.dbclass))
            nc.append(conn._connectionXML)
        print(ET.tostring(root))
        return cls(root)

I don't actually know the rules for what makes a good 'name' -- it appears to be dbclass + a 28 character random string ('postgres.1of3kl00aoax5d1a1ejma1397430'), it's definitely not a UUID though

@t8y8
Copy link
Contributor Author

t8y8 commented Aug 4, 2016

Alright, pushed my attempt.

@RussTheAerialist I'm not sure if it's overcomplicating things or not, but it feels like this logic could live in an, bleck, DatasourceBuilder, or something like that. It could take in a version parameter and know how to build 9.X vs 10.X as well.

Of course, none of that addresses the problem that these are 'bare' datasources without relations, metadata-records, or fields.

Thoughts?

@@ -63,7 +66,15 @@ def _column_object_from_metadata_xml(metadata_xml):
return _ColumnObjectReturnTuple(field_object.id, field_object)


def make_unique_name(dbclass):
rand_part = ''.join(random.choice(
Copy link
Contributor

Choose a reason for hiding this comment

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

Our unique names are a hash based on the current time, so we should probably do that rather than random data. I can find the relevant code if you need it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please!

@graysonarts
Copy link
Contributor

🚀 this will need to be changed once I get all of the editor+physical+logical splitting done but for now, yay!

@t8y8
Copy link
Contributor Author

t8y8 commented Aug 15, 2016

Awesome! At least it'll let us play with some examples, and the tests will stay valid.

@t8y8 t8y8 merged commit 8b8f351 into tableau:development Aug 15, 2016
@t8y8 t8y8 deleted the 68-feature-create-connection branch August 15, 2016 23:08
@graysonarts graysonarts modified the milestone: 2016.08 release Aug 29, 2016
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