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

Import connections from a file #15177

Merged
merged 4 commits into from
Apr 5, 2021
Merged

Conversation

natanweinberger
Copy link
Contributor

This PR adds the ability to import connections from a file.

This implementation enables basic import functionality, mirroring what already exists for variables. There's room for additional features in future (as proposed in #9907 discussion), but I believe it's reasonable to add those extra features as incremental improvements in a future PR. For example:

  • handling conflict disposition consistently for variables and connections
  • accepting input from stdin

Currently, in the event of a conflict, the connection to be imported is skipped and a message is printed to stdout indicating to the user that there's a conflict. Any connections without conflicts are still imported.

closes: #9855

@boring-cyborg
Copy link

boring-cyborg bot commented Apr 3, 2021

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst)
Here are some useful points:

  • Pay attention to the quality of your code (flake8, pylint and type annotations). Our pre-commits will help you with that.
  • In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example DAG that shows how users should use it.
  • Consider using Breeze environment for testing locally, it’s a heavy docker but it ships with a working Airflow and a lot of integrations.
  • Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
  • Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
  • Be sure to read the Airflow Coding style.
    Apache Airflow is a community-driven project and together we are making it better 🚀.
    In case of doubts contact the developers at:
    Mailing List: dev@airflow.apache.org
    Slack: https://s.apache.org/airflow-slack

When a connections file contains collisions with existing connections,
skip them and print a message to stdout indicating that the connection
was not imported.
@dimberman dimberman merged commit 7cadb63 into apache:master Apr 5, 2021
@boring-cyborg
Copy link

boring-cyborg bot commented Apr 5, 2021

Awesome work, congrats on your first merged pull request!

@dimberman
Copy link
Contributor

Congrats @natanweinberger !

@mudravrik
Copy link

Sorry, @natanweinberger , it must be wrong place to ask such question, but I tried this feature and get following error:

Traceback (most recent call last):
  File "/home/airflow/.local/bin/airflow", line 8, in <module>
    sys.exit(main())
  File "/home/airflow/.local/lib/python3.8/site-packages/airflow/__main__.py", line 40, in main
    args.func(args)
  File "/home/airflow/.local/lib/python3.8/site-packages/airflow/cli/cli_parser.py", line 48, in command
    return func(*args, **kwargs)
  File "/home/airflow/.local/lib/python3.8/site-packages/airflow/utils/cli.py", line 91, in wrapper
    return f(*args, **kwargs)
  File "/home/airflow/.local/lib/python3.8/site-packages/airflow/cli/commands/connection_command.py", line 244, in connections_import
    _import_helper(args.file)
  File "/home/airflow/.local/lib/python3.8/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'

Am I doing something completly wrong?

@natanweinberger
Copy link
Contributor Author

Hey @mudravrik, I see the issue. Thanks for the heads up, I will fix this and open a new PR shortly.

@MM-Lehmann
Copy link

How did this not get a documentation entry? Any definition of how the json must look like? Even the sample.json mentioned in the tests is not in this PR, is it?

@potiuk
Copy link
Member

potiuk commented Aug 25, 2021

If you really cared to ask this kind of question @MM-Lehmann, then I believe because apparently you wanted to hear the answer rather than show your superiority over those who volunteer their time so that you can get great FREE software.

So here it is.

My one answer is - because people are humans and when they try to improve something, they sometimes make mistakes (as opposed to people who do not do do much so they do not do mistakes either).

Or maybe another answer - because you weren't around with helpful comment like that when it was discussed @MM-Lehmann . I am sure if you were around such mistakes would not have happened.

Please next time be sure to follow up the changed PRs and catch such omissions early.

Airflow PRs can be commented and suggested by everyone. Please do so next time before merging!

You are most welcome.

@potiuk
Copy link
Member

potiuk commented Aug 25, 2021

'And answering your other question @MM-Lehmann - in case you have not noticed the "import" command is counterpart of the "export" command https://airflow.apache.org/docs/apache-airflow/stable/cli-and-env-variables-ref.html#export which result you can inspect and check the format.

If you really think the format needs to be described - you are absolutely invited to contribute documentation for both export and import after reverse engineering it! I am not kidding. I think this might be valuable contribution and great way to repay for all the countless - often unpaid hours other contributors spent on building airflow (which you can now use for free).

@ishnmu
Copy link

ishnmu commented Oct 21, 2022

Hi @natanweinberger , can we have a --override option which updates the connection when there is a conflict ?

@natanweinberger
Copy link
Contributor Author

natanweinberger commented Jan 4, 2023

Hi @natanweinberger , can we have a --override option which updates the connection when there is a conflict ?

Good idea @ishnmu, I just opened a PR that adds this functionality.

@natanweinberger
Copy link
Contributor Author

#28738 has been merged in, allowing connections to be overwritten. Thanks for the suggestion, @ishnmu!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Import connections/variables from a file
7 participants