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

Use MariaDB client binaries in arm64 image for support MySQL backend #29519

Merged
merged 3 commits into from
Feb 22, 2023

Conversation

Taragolis
Copy link
Contributor

@Taragolis Taragolis commented Feb 13, 2023

According to https://mariadb.com/kb/en/mariadb-clientserver-tcp-protocol/ MariaDB client use the same protocol as use MySQL.

This PoC use binaries which included in standard Debian repository only for ARM64 images, AMD64/x86_64 still use Oracle MySQL repository, see all supported options

Airflow starts almost fine in breeze with MySQL 8.0 backend, except:

  • There is not native ARM image for 5.7, so disable it in breeze. Yeah it is still an option with emulation AMD64, but personally I do not think it is make sense to add support for something which almost EOL.
  • I have a DAG with start date in 1900, due to the fact we use TIMESTAMP for MySQL as result there is no possible to run DAG with start date lower than 1970-01-01. It is probably something new for me, rather than actual problem.

Some additionals:

  • Potentially I could miss some part which also need to be changed.
  • This PR create "chicken or egg" problem, users can't install provider in old Airflow Image without extend/build own image with MariaDB client as well as need install python packages manually if use old provider.
  • This changes do not add support MariaDB as backend

@Taragolis Taragolis marked this pull request as draft February 13, 2023 23:20
@potiuk
Copy link
Member

potiuk commented Feb 13, 2023

Interesting. Might works since ARM image is experimental anyway :)

Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

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

nice one. I hope it will pass :)

@Taragolis
Copy link
Contributor Author

  × python setup.py bdist_wheel did not run successfully.
  │ exit code: 1
  ╰─> [16 lines of output]
      running bdist_wheel
      running build
      running build_ext
      building 'pyodbc' extension
      creating build
      creating build/temp.linux-x86_64-3.7
      creating build/temp.linux-x86_64-3.7/src
      gcc -pthread -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -fPIC -DPYODBC_VERSION=4.0.32 -I/usr/local/include/python3.7m -c src/buffer.cpp -o build/temp.linux-x86_64-3.7/src/buffer.o -Wno-write-strings
      In file included from /usr/include/sql.h:19,
                       from src/pyodbc.h:56,
                       from src/buffer.cpp:12:
      /usr/include/sqltypes.h:56:10: fatal error: unixodbc.h: No such file or directory
         56 | #include "unixodbc.h"
            |          ^~~~~~~~~~~~
      compilation terminated.
      error: command 'gcc' failed with exit status 1
      [end of output]
  
  note: This error originates from a subprocess, and is likely not a problem with pip.
  ERROR: Failed building wheel for pyodbc

I guess changes for this PR have led to rebuild image almost from scratch (without caches) and get unixodbc 2.3.11 and other related packages which is not work with pyodbc 4.0.32, and unfortunetly pyodbc==4.0.32 is a constraint for Airflow 2.3.0

Same as described here:

@potiuk
Copy link
Member

potiuk commented Feb 14, 2023

We are aware of the problem and are working on an updated 2.3.11 package for unixodbc-dev, ASAP.

Regards,
David

@Taragolis
Copy link
Contributor Author

In the morning I think how to fix this sometimes "just wait" it is a better tactic 🤣

@Taragolis Taragolis force-pushed the use-mariadb-libraries-on-arm branch from f875a0f to 0d0f6f2 Compare February 14, 2023 22:25
@potiuk
Copy link
Member

potiuk commented Feb 14, 2023

In the morning I think how to fix this sometimes "just wait" it is a better tactic 🤣

Oh absolutely. But you never know when.

@Taragolis
Copy link
Contributor Author

New test for collection of flaky tests: TestDbtCloudRunJobOperator.test_execute_wait_for_termination[default_account-3-timeout]. I can't say that failure of this test happens on the regular basis but this is not a first time when I notice that

            if job_run_status in DbtCloudJobRunStatus.TERMINAL_STATUSES.value:
                assert mock_get_job_run.call_count == 1
            else:
                # When the job run status is not in a terminal status or "Success", the operator will
                # continue to call ``get_job_run()`` until a ``timeout`` number of seconds has passed
                # (3 seconds for this test).  Therefore, there should be 4 calls of this function: one
                # initially and 3 for each check done at a 1 second interval.
>               assert mock_get_job_run.call_count == 4
E               AssertionError: assert 2 == 4
E                +  where 2 = <MagicMock name='get_job_run' id='140182627981488'>.call_count

@Taragolis
Copy link
Contributor Author

Oh absolutely. But you never know when.

Well, this not yet a problem on our main but may be we should temporary pin unixodbc and related packages for specific version or bump constraints for older airflow to pyodbc==4.0.35 ?

@Taragolis Taragolis force-pushed the use-mariadb-libraries-on-arm branch from 0d0f6f2 to 88fde16 Compare February 22, 2023 00:13
@Taragolis Taragolis changed the title PoC: Use MariaDB client binaries in arm64 image for support MySQL backend Use MariaDB client binaries in arm64 image for support MySQL backend Feb 22, 2023
@Taragolis Taragolis marked this pull request as ready for review February 22, 2023 08:20
@Taragolis Taragolis requested a review from kaxil as a code owner February 22, 2023 08:20
@Taragolis
Copy link
Contributor Author

And finally it is green!

Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

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

Fingers crossed 🤞

@Taragolis
Copy link
Contributor Author

Shall we? Or should be better that someone else look on this changes?

@Taragolis
Copy link
Contributor Author

And I also run some tests with @pytest.mark.backend("mysql") locally

breeze shell --backend mysql -M 8 --db-reset
...

root@1b371384f7b3:/opt/airflow# pytest tests/operators/test_generic_transfer.py::TestMySql
============================================================== test session starts ===============================================================
platform linux -- Python 3.7.16, pytest-7.2.1, pluggy-1.0.0 -- /usr/local/bin/python
cachedir: .pytest_cache
rootdir: /opt/airflow, configfile: pytest.ini
plugins: xdist-3.2.0, anyio-3.6.2, time-machine-2.9.0, instafail-0.4.2, httpx-0.21.3, timeouts-1.2.1, requests-mock-1.10.0, asyncio-0.20.3, capture-warnings-0.0.4, rerunfailures-11.1.1, cov-4.0.0
asyncio: mode=strict
setup timeout: 0.0s, execution timeout: 0.0s, teardown timeout: 0.0s
collected 3 items                                                                                                                                

tests/operators/test_generic_transfer.py::TestMySql::test_mysql_to_mysql[mysqlclient] PASSED                                               [ 33%]
tests/operators/test_generic_transfer.py::TestMySql::test_mysql_to_mysql[mysql-connector-python] PASSED                                    [ 66%]
tests/operators/test_generic_transfer.py::TestMySql::test_mysql_to_mysql_replace PASSED                                                    [100%]

@potiuk potiuk merged commit ff8fae1 into apache:main Feb 22, 2023
@potiuk
Copy link
Member

potiuk commented Feb 22, 2023

Looks cool!

@Taragolis Taragolis deleted the use-mariadb-libraries-on-arm branch February 22, 2023 17:14
potiuk added a commit to potiuk/airflow that referenced this pull request Jul 6, 2023
The change apache#29519 added experimental MariaDB client support for
MySQL for ARM images but we missed that in the changelog.
potiuk added a commit that referenced this pull request Jul 6, 2023
The change #29519 added experimental MariaDB client support for
MySQL for ARM images but we missed that in the changelog.
ephraimbuddy pushed a commit that referenced this pull request Jul 6, 2023
The change #29519 added experimental MariaDB client support for
MySQL for ARM images but we missed that in the changelog.

(cherry picked from commit ef75a3a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:dev-tools area:production-image Production image improvements and fixes area:providers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants