Skip to content

Conversation

@GrayFlash
Copy link
Member

@GrayFlash GrayFlash commented Jun 1, 2022

closes #353

  • postgres
  • mysql
  • mssql
  • tableau

@GrayFlash GrayFlash self-assigned this Jun 1, 2022
@GrayFlash GrayFlash requested a review from StewartJingga June 1, 2022 14:51
@GrayFlash GrayFlash marked this pull request as ready for review June 1, 2022 15:34
@GrayFlash
Copy link
Member Author

@StewartJingga I have made changes to add instance_label as a part of config for the above 4 extractors. Wanted to know if sessionID instead of password&username is to be added for tableau. I am not sure if sql extractors will also need a sessionID since we use connection string there.

@mabdh
Copy link
Member

mabdh commented Jun 2, 2022

@StewartJingga I have made changes to add instance_label as a part of config for the above 4 extractors. Wanted to know if sessionID instead of password&username is to be added for tableau. I am not sure if sql extractors will also need a sessionID since we use connection string there.

It is called auth token in tableau and not session id. If metabase has it, nothing is stopping us to add the similar flow for tableau.

@GrayFlash
Copy link
Member Author

@mabdh @StewartJingga created a separate issue for it here. #358

@ravisuhag
Copy link
Member

@GrayFlash @StewartJingga Do you think we can find a better name for this config instead of instance_label ?

@GrayFlash
Copy link
Member Author

can we use record_label since we use it to identify records being extracted as a part of urn?? wdyt @ravisuhag @StewartJingga

@mabdh
Copy link
Member

mabdh commented Jun 7, 2022

What about namespace?

@GrayFlash
Copy link
Member Author

@mabdh @StewartJingga @ravisuhag even namespace seems fine in terms of conveying its purpose.

@StewartJingga
Copy link
Contributor

@mabdh @StewartJingga @ravisuhag even namespace seems fine in terms of conveying its purpose.

namespace works, but I feel like it has a chance of having another meaning on some source. (e.g. tableau).

@mabdh
Copy link
Member

mabdh commented Jun 7, 2022

namespace naming is actually inspired by this URN IETF doc. Namespace Specific String (NSS)

@GrayFlash
Copy link
Member Author

@mabdh makes sense.

@StewartJingga @ravisuhag should we use namespace then??

@ravisuhag
Copy link
Member

We can use identifier as well to address the concern @StewartJingga has. cc @mabdh @GrayFlash

@ravisuhag ravisuhag merged commit b94d77f into main Jun 7, 2022
@ravisuhag ravisuhag deleted the instance-label-extractors branch June 7, 2022 11:12
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.

feat: add instance_label to extractors

5 participants