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

CLI Connection import command not work #16374

Closed
ravog opened this issue Jun 10, 2021 · 5 comments
Closed

CLI Connection import command not work #16374

ravog opened this issue Jun 10, 2021 · 5 comments
Labels
kind:bug This is a clearly a bug

Comments

@ravog
Copy link

ravog commented Jun 10, 2021

Apache Airflow version:

Kubernetes version (if you are using kubernetes) (use kubectl version):

Environment:

  • Cloud provider or hardware configuration:
  • OS (e.g. from /etc/os-release):
  • Kernel (e.g. uname -a):
  • Install tools:
  • Others:

What happened:

File "/home/airflow/.local/lib/python3.6/site-packages/airflow/cli/commands/connection_command.py", line 272, in _import_helper
key: value for key, value in conn_values.items() if key in allowed_fields
AttributeError: 'Connection' object has no attribute 'items'

I just use the file generated from connections export command

What you expected to happen:

How to reproduce it:

Anything else we need to know:

@ravog ravog added the kind:bug This is a clearly a bug label Jun 10, 2021
@DDullahan
Copy link

I met the same problem and checked the source code.

In airflow/cli/commands/connection_command.py it loads connections file as a dict, conn_id for key, Connection type for value.
After that, it calls items() on Connection type which doesn't have.

def _import_helper(file_path):
    """Helps import connections from a file"""
    connections_dict = load_connections_dict(file_path)
    with create_session() as session:
        for conn_id, conn_values in connections_dict.items():
            # type(conn_id) is str
            # type(conn_values) is airflow.models.connection.Connection

            if session.query(Connection).filter(Connection.conn_id == conn_id).first():
                print(f'Could not import connection {conn_id}: connection already exists.')
                continue

            allowed_fields = [
                'extra',                                                                                                              
                'description',
                'conn_id',
                'login',                                                                                                                
                'conn_type',
                'host',                                                                                                                
                'password',
                'schema',
                'port',
                'uri',                                                                                                                 
                'extra_dejson',                                                                                                     
            ]
            filtered_connection_values = {
                key: value for key, value in conn_values.items() if key in allowed_fields
            }
            connection = _create_connection(conn_id, filtered_connection_values)
            session.add(connection)
            session.commit()
            print(f'Imported connection {conn_id}')      

I guess in the previous version, method load_connections_dict just loads connections file as a json object. In this situation, this code can run correctly.

Maybe you should use airflow connections create for now.

@uranusjr
Copy link
Member

Weirdly enough, from what I can tell with git blame, load_connections_dict has always returned a Connection object during the entire lifetime of this command (only two months old; load_connections_dict was last changed 8 months ago). Maybe @natanweinberger could provide some background why the implementation was made this way; there was actually a unit test, but the load_connections_dict mock is configured to not do the right thing from what I can tell. (See #15177)

@natanweinberger
Copy link
Contributor

natanweinberger commented Jun 11, 2021

Yes, that's right. I made a last minute change that resulted in a mistake which wasn't caught in the unit test. This was just fixed in #15425 this morning!

Check out the description of #15425 for a full explanation of the bug, the root cause and the fix.

@uranusjr
Copy link
Member

You borrowed Guido’s time machine for this one, didn’t you 😉 So I think this can be closed?

@natanweinberger
Copy link
Contributor

😁

Yes, it can be closed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug This is a clearly a bug
Projects
None yet
Development

No branches or pull requests

5 participants