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

CatalogIdparameter is missing while creating Relation #414

Open
weiter10 opened this issue Sep 15, 2023 · 5 comments
Open

CatalogIdparameter is missing while creating Relation #414

weiter10 opened this issue Sep 15, 2023 · 5 comments
Labels
bug Something isn't working question Further information is requested

Comments

@weiter10
Copy link

Hi,

I'm experimenting an issue while creating a relation. I use this statement:

{% set relation = api.Relation.create(schema = database, identifier = table,) %}

Which returns the following error:

Parameter validation failed:

Invalid type for parameter CatalogId, value: None, type: <class 'NoneType'>, valid types: <class 'str'>

Packages:
dbt-core==1.6.2
dbt-athena-community==1.6.1

[Research]

With dbt-athena-community==1.6.0 it runs without errors.

I supposed that this error is raised by boto3 which indicates that CatalogIdparameter is missing.

Reading the documentation of Relation class from dbt, I have noticed that Relation class expects three parameters:

class Relation:
  def create(database=None, schema=None, identifier=None,
             type=None):
  """
    database (optional): The name of the database for this relation
    schema (optional): The name of the schema (or dataset, if on BigQuery) for this relation
    identifier (optional): The name of the identifier for this relation
    type (optional): Metadata about this relation, eg: "table", "view", "cte"
  """

Well, if I add the database parameter (that is Glue Catalog) to the statement:

{% set relation = api.Relation.create(schema = database, identifier = table, database='awsdatacatalog') %}

It works ok.


Will this behaviour be maintained in nexts dbt-athena-communit versions, or it's a bug and it will return the original behaviour, (that is, it won't be needed database='awsdatacatalog')?

Thank you so much for developing this software, I use it with Iceberg tables and It works very very well.

@jessedobbelaere
Copy link
Contributor

I think this was introduced because of #370 .
I tried to harden the code in #411 against this issue, but realized in the end that I could update the relation to specify both schema and database to fix the issue. I suppose this is the way to go to always specify both? Or would you rather see the database as optional?

@weiter10
Copy link
Author

Or would you rather see the database as optional?

I think it could be nice ( see database as optional with default value awsdatacatalog.
If cross account is necessary, database could be replaced :)

Thank you

@Jrmyy
Copy link
Contributor

Jrmyy commented Oct 9, 2023

It seems that indeed now we support cross-account queries we must pass the database parameter. If I check how we create relations, we always pass the database parameter

Maybe it is worth updating the code to set to awsdatacatalog if the database is None.
WDYT @nicor88 @svdimchenko ?

@Jrmyy Jrmyy added bug Something isn't working question Further information is requested labels Oct 9, 2023
@nicor88
Copy link
Contributor

nicor88 commented Oct 9, 2023

I'm able to do cross-account queries at the moment using the latest version of the adapter, but using lakeformation/RAM and database links.
The trick is that the database link gets created with the catalog id of the current account, but it points under the hood to a database that is in another account....

@nicor88
Copy link
Contributor

nicor88 commented Oct 15, 2023

@Jrmyy @jessedobbelaere

Maybe it is worth updating the code to set to awsdatacatalog if the database is None

Will this still work cross account? The catalog name is 'awsdatacatalog' indipendently from the account.
But what change is catalog_id (that is the account ID). Maybe we should support catalog ID as optional? If not passed the current account is used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants