Fix #1821: PyHive is incompatible with sqlalchemy-trino

Describe your changes :
See #1821

Type of change :

[x] Improvement

Reviewers
@harshach @ayush-shah @pmbrull

@amiorin instead of throwing an error, I think we can use pyhive for both hive and trino.
Pyhive started supporting trino https://github.com/dropbox/PyHive/pull/381
just change the https://github.com/open-metadata/OpenMetadata/blob/main/ingestion/setup.py#L86
to sqlalchemy-hive as the dependency. I think that will work.
@amiorin can you test that, if that works then we don't need to throw this error message to the user and remove the trino dependency.
cc @ayush-shah

Also, because of these kind of errors we prepared https://github.com/open-metadata/OpenMetadata/pull/1730. When possible (prod airflow pipelines), let's try to redirect users to DockerOperator. I'll prepare some docs shortly and let you know!

@amiorin instead of throwing an error, I think we can use pyhive for both hive and trino. Pyhive started supporting trino dropbox/PyHive#381 just change the https://github.com/open-metadata/OpenMetadata/blob/main/ingestion/setup.py#L86 to sqlalchemy-hive as the dependency. I think that will work. @amiorin can you test that, if that works then we don't need to throw this error message to the user and remove the trino dependency. cc @ayush-shah

It doesn't work for me. It's using sqlalchemy_presto.py even if the connection is trino://user@localhost:8080/tpcds/tiny?.
╰─$ metadata ingest -c examples/workflows/trino.json
[2021-12-19 01:09:18,535] INFO     {metadata.ingestion.source.trino:58} - trino://user@localhost:8080/tpcds/tiny?
[2021-12-19 01:09:18,535] INFO     {metadata.ingestion.source.sql_source:361} - Parsing Data Models
[2021-12-19 01:09:21,727] INFO     {metadata.ingestion.source.sql_source:64} - Table Scanned: local_trino.call_center
[2021-12-19 01:09:21,727] ERROR    {metadata.ingestion.source.sql_source:148} - Table Description Error :
/Users/amiorin/.pyenv/versions/3.9.7/envs/openmetadata/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'char(16)' of column 'cc_call_center_id'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/openmetadata/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'varchar(50)' of column 'cc_name'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/openmetadata/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'varchar(50)' of column 'cc_class'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/openmetadata/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'char(20)' of column 'cc_hours'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/openmetadata/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'varchar(40)' of column 'cc_manager'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/openmetadata/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'char(50)' of column 'cc_mkt_class'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/openmetadata/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'varchar(100)' of column 'cc_mkt_desc'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/openmetadata/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'varchar(40)' of column 'cc_market_manager'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/openmetadata/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'varchar(50)' of column 'cc_division_name'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/openmetadata/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'char(50)' of column 'cc_company_name'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/openmetadata/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'char(10)' of column 'cc_street_number'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/openmetadata/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'varchar(60)' of column 'cc_street_name'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/openmetadata/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'char(15)' of column 'cc_street_type'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/openmetadata/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'char(10)' of column 'cc_suite_number'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/openmetadata/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'varchar(60)' of column 'cc_city'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/openmetadata/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'varchar(30)' of column 'cc_county'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/openmetadata/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'char(2)' of column 'cc_state'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/openmetadata/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'char(10)' of column 'cc_zip'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/openmetadata/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'varchar(20)' of column 'cc_country'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/openmetadata/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'decimal(5,2)' of column 'cc_gmt_offset'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/openmetadata/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'decimal(5,2)' of column 'cc_tax_percentage'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
[2021-12-19 01:09:24,818] ERROR    {metadata.ingestion.source.sql_source:559} - CompileError("Can't generate DDL for NullType(); did you forget to specify a type on this Column?"): call_center Can't generate DDL for NullType(); did you forget to specify a type on this Column?
[2021-12-19 01:09:24,818] ERROR    {metadata.ingestion.source.sql_source:296} - 1 validation error for Table
columns
  none is not an allowed value (type=type_error.none.not_allowed)

@amiorin can you give me steps to reproduce the issue you are facing. Here is what I've done so far

Create new python env
Install openmetadata-ingestion, presto connector

4. install openmetadata-ingestion, trino connector

5. Run Trino connector to fetch metadata

This works fine for me. Can you share the details how you are able to reproduce the error you are facing.

I have tried to reproduce it from scratch.
╭─amiorin@silicon ~/code/OpenMetadata/ingestion ‹main*›
╰─$ pyenv virtualenv 3.9.7 test
Looking in links: /var/folders/55/c_wsr21n74z8pfp_k4spntvh0000gn/T/tmp36d5q39e
Requirement already satisfied: setuptools in /Users/amiorin/.pyenv/versions/3.9.7/envs/test/lib/python3.9/site-packages (57.4.0)
Requirement already satisfied: pip in /Users/amiorin/.pyenv/versions/3.9.7/envs/test/lib/python3.9/site-packages (21.2.3)
╭─amiorin@silicon ~/code/OpenMetadata/ingestion ‹main*›
╰─$ pip install -e '.[hive]'
Obtaining file:///Users/amiorin/code/OpenMetadata/ingestion
Collecting pydantic[email]>=1.7.2
  Using cached pydantic-1.8.2-py3-none-any.whl (126 kB)
Collecting cryptography
  Using cached cryptography-36.0.1-cp36-abi3-macosx_10_10_universal2.whl (4.8 MB)
Collecting click>=7.1.1
  Using cached click-8.0.3-py3-none-any.whl (97 kB)
Collecting google-auth>=1.33.0
  Using cached google_auth-2.3.3-py2.py3-none-any.whl (155 kB)
Collecting typing_extensions>=3.7.4
  Using cached typing_extensions-4.0.1-py3-none-any.whl (22 kB)
Collecting sqlalchemy>=1.3.24
  Using cached SQLAlchemy-1.4.28.tar.gz (8.0 MB)
Collecting idna<3,>=2.5
  Using cached idna-2.10-py2.py3-none-any.whl (58 kB)
Collecting typing-inspect
  Using cached typing_inspect-0.7.1-py3-none-any.whl (8.4 kB)
Collecting requests~=2.26
  Using cached requests-2.26.0-py2.py3-none-any.whl (62 kB)
Collecting PyYAML
  Using cached PyYAML-6.0-cp39-cp39-macosx_11_0_arm64.whl (173 kB)
Collecting google>=3.0.0
  Using cached google-3.0.0-py2.py3-none-any.whl (45 kB)
Collecting sql-metadata~=2.0.0
  Using cached sql_metadata-2.0.0-py3-none-any.whl (16 kB)
Collecting python-dateutil>=2.8.1
  Using cached python_dateutil-2.8.2-py2.py3-none-any.whl (247 kB)
Collecting wheel~=0.36.2
  Using cached wheel-0.36.2-py2.py3-none-any.whl (35 kB)
Collecting mypy_extensions>=0.4.3
  Using cached mypy_extensions-0.4.3-py2.py3-none-any.whl (4.5 kB)
Collecting python-jose==3.3.0
  Using cached python_jose-3.3.0-py2.py3-none-any.whl (33 kB)
Collecting email-validator>=1.0.3
  Using cached email_validator-1.1.3-py2.py3-none-any.whl (18 kB)
Collecting commonregex
  Using cached commonregex-1.5.4.tar.gz (2.8 kB)
Collecting thrift-sasl==0.4.3
  Using cached thrift_sasl-0.4.3-py2.py3-none-any.whl (8.3 kB)
Collecting sasl==0.3.1
  Using cached sasl-0.3.1-cp39-cp39-macosx_12_0_arm64.whl
Collecting thrift~=0.13.0
  Using cached thrift-0.13.0-cp39-cp39-macosx_12_0_arm64.whl
Collecting openmetadata-sqlalchemy-hive==0.2.0
  Using cached openmetadata_sqlalchemy_hive-0.2.0-py3-none-any.whl (52 kB)
Collecting future
  Using cached future-0.18.2.tar.gz (829 kB)
Collecting ecdsa!=0.15
  Using cached ecdsa-0.17.0-py2.py3-none-any.whl (119 kB)
Collecting rsa
  Using cached rsa-4.8-py3-none-any.whl (39 kB)
Collecting pyasn1
  Using cached pyasn1-0.4.8-py2.py3-none-any.whl (77 kB)
Collecting six
  Using cached six-1.16.0-py2.py3-none-any.whl (11 kB)
Collecting pure-sasl>=0.6.2
  Using cached pure_sasl-0.6.2-py3-none-any.whl
Collecting dnspython>=1.15.0
  Using cached dnspython-2.1.0-py3-none-any.whl (241 kB)
Collecting beautifulsoup4
  Using cached beautifulsoup4-4.10.0-py3-none-any.whl (97 kB)
Collecting pyasn1-modules>=0.2.1
  Using cached pyasn1_modules-0.2.8-py2.py3-none-any.whl (155 kB)
Requirement already satisfied: setuptools>=40.3.0 in /Users/amiorin/.pyenv/versions/3.9.7/envs/test/lib/python3.9/site-packages (from google-auth>=1.33.0->openmetadata-ingestion==0.5.0.dev0) (57.4.0)
Collecting cachetools<5.0,>=2.0.0
  Using cached cachetools-4.2.4-py3-none-any.whl (10 kB)
Collecting certifi>=2017.4.17
  Using cached certifi-2021.10.8-py2.py3-none-any.whl (149 kB)
Collecting charset-normalizer~=2.0.0
  Using cached charset_normalizer-2.0.9-py3-none-any.whl (39 kB)
Collecting urllib3<1.27,>=1.21.1
  Using cached urllib3-1.26.7-py2.py3-none-any.whl (138 kB)
Collecting sqlparse<0.5.0,>=0.4.1
  Using cached sqlparse-0.4.2-py3-none-any.whl (42 kB)
Collecting soupsieve>1.2
  Using cached soupsieve-2.3.1-py3-none-any.whl (37 kB)
Collecting cffi>=1.12
  Using cached cffi-1.15.0-cp39-cp39-macosx_11_0_arm64.whl (173 kB)
Collecting pycparser
  Using cached pycparser-2.21-py2.py3-none-any.whl (118 kB)
Using legacy 'setup.py install' for sqlalchemy, since package 'wheel' is not installed.
Using legacy 'setup.py install' for commonregex, since package 'wheel' is not installed.
Using legacy 'setup.py install' for future, since package 'wheel' is not installed.
Installing collected packages: typing-extensions, soupsieve, six, pycparser, pyasn1, idna, dnspython, urllib3, sqlparse, rsa, pydantic, pyasn1-modules, mypy-extensions, email-validator, ecdsa, charset-normalizer, cffi, certifi, cachetools, beautifulsoup4, wheel, typing-inspect, thrift, sqlalchemy, sql-metadata, requests, PyYAML, python-jose, python-dateutil, pure-sasl, google-auth, google, future, cryptography, commonregex, click, thrift-sasl, sasl, openmetadata-sqlalchemy-hive, openmetadata-ingestion
    Running setup.py install for sqlalchemy ... done
    Running setup.py install for future ... done
    Running setup.py install for commonregex ... done
  Running setup.py develop for openmetadata-ingestion
Successfully installed PyYAML-6.0 beautifulsoup4-4.10.0 cachetools-4.2.4 certifi-2021.10.8 cffi-1.15.0 charset-normalizer-2.0.9 click-8.0.3 commonregex-1.5.4 cryptography-36.0.1 dnspython-2.1.0 ecdsa-0.17.0 email-validator-1.1.3 future-0.18.2 google-3.0.0 google-auth-2.3.3 idna-2.10 mypy-extensions-0.4.3 openmetadata-ingestion-0.5.0.dev0 openmetadata-sqlalchemy-hive-0.2.0 pure-sasl-0.6.2 pyasn1-0.4.8 pyasn1-modules-0.2.8 pycparser-2.21 pydantic-1.8.2 python-dateutil-2.8.2 python-jose-3.3.0 requests-2.26.0 rsa-4.8 sasl-0.3.1 six-1.16.0 soupsieve-2.3.1 sql-metadata-2.0.0 sqlalchemy-1.4.28 sqlparse-0.4.2 thrift-0.13.0 thrift-sasl-0.4.3 typing-extensions-4.0.1 typing-inspect-0.7.1 urllib3-1.26.7 wheel-0.36.2
WARNING: You are using pip version 21.2.3; however, version 21.3.1 is available.
You should consider upgrading via the '/Users/amiorin/.pyenv/versions/3.9.7/envs/test/bin/python3.9 -m pip install --upgrade pip' command.
╭─amiorin@silicon ~/code/OpenMetadata/ingestion ‹main*›
╰─$ metadata ingest -c examples/workflows/trino.json
Traceback (most recent call last):
  File "/Users/amiorin/.pyenv/versions/test/bin/metadata", line 33, in <module>
    sys.exit(load_entry_point('openmetadata-ingestion', 'console_scripts', 'metadata')())
  File "/Users/amiorin/.pyenv/versions/3.9.7/envs/test/lib/python3.9/site-packages/click/core.py", line 1128, in __call__
    return self.main(*args, **kwargs)
  File "/Users/amiorin/.pyenv/versions/3.9.7/envs/test/lib/python3.9/site-packages/click/core.py", line 1053, in main
    rv = self.invoke(ctx)
  File "/Users/amiorin/.pyenv/versions/3.9.7/envs/test/lib/python3.9/site-packages/click/core.py", line 1659, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/Users/amiorin/.pyenv/versions/3.9.7/envs/test/lib/python3.9/site-packages/click/core.py", line 1395, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/Users/amiorin/.pyenv/versions/3.9.7/envs/test/lib/python3.9/site-packages/click/core.py", line 754, in invoke
    return __callback(*args, **kwargs)
  File "/Users/amiorin/code/OpenMetadata/ingestion/src/metadata/cmd.py", line 75, in ingest
    workflow = Workflow.create(workflow_config)
  File "/Users/amiorin/code/OpenMetadata/ingestion/src/metadata/ingestion/api/workflow.py", line 150, in create
    return cls(config)
  File "/Users/amiorin/code/OpenMetadata/ingestion/src/metadata/ingestion/api/workflow.py", line 68, in __init__
    self.source: Source = source_class.create(
  File "/Users/amiorin/code/OpenMetadata/ingestion/src/metadata/ingestion/source/trino.py", line 62, in create
    return cls(config, metadata_config, ctx)
  File "/Users/amiorin/code/OpenMetadata/ingestion/src/metadata/ingestion/source/trino.py", line 56, in __init__
    super().__init__(config, metadata_config, ctx)
  File "/Users/amiorin/code/OpenMetadata/ingestion/src/metadata/ingestion/source/sql_source.py", line 168, in __init__
    self.engine = create_engine(
  File "<string>", line 2, in create_engine
  File "/Users/amiorin/.pyenv/versions/3.9.7/envs/test/lib/python3.9/site-packages/sqlalchemy/util/deprecations.py", line 309, in warned
    return fn(*args, **kwargs)
  File "/Users/amiorin/.pyenv/versions/3.9.7/envs/test/lib/python3.9/site-packages/sqlalchemy/engine/create.py", line 534, in create_engine
    entrypoint = u._get_entrypoint()
  File "/Users/amiorin/.pyenv/versions/3.9.7/envs/test/lib/python3.9/site-packages/sqlalchemy/engine/url.py", line 661, in _get_entrypoint
    cls = registry.load(name)
  File "/Users/amiorin/.pyenv/versions/3.9.7/envs/test/lib/python3.9/site-packages/sqlalchemy/util/langhelpers.py", line 341, in load
    return impl.load()
  File "/Users/amiorin/.pyenv/versions/3.9.7/lib/python3.9/importlib/metadata.py", line 77, in load
    module = import_module(match.group('module'))
  File "/Users/amiorin/.pyenv/versions/3.9.7/lib/python3.9/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1030, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1007, in _find_and_load
  File "<frozen importlib._bootstrap>", line 986, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 680, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 850, in exec_module
  File "<frozen importlib._bootstrap>", line 228, in _call_with_frames_removed
  File "/Users/amiorin/.pyenv/versions/3.9.7/envs/test/lib/python3.9/site-packages/pyhive/sqlalchemy_trino.py", line 21, in <module>
    from pyhive import trino
  File "/Users/amiorin/.pyenv/versions/3.9.7/envs/test/lib/python3.9/site-packages/pyhive/trino.py", line 18, in <module>
    from pyhive.presto import Connection as PrestoConnection, Cursor as PrestoCursor, PrestoParamEscaper
  File "/Users/amiorin/.pyenv/versions/3.9.7/envs/test/lib/python3.9/site-packages/pyhive/presto.py", line 14, in <module>
    from presto_types_parser import build_row_parser
ModuleNotFoundError: No module named 'presto_types_parser'
╭─amiorin@silicon ~/code/OpenMetadata/ingestion ‹main*›
╰─$ pip install presto_types_parser
Collecting presto_types_parser
  Using cached presto_types_parser-0.0.2-py3-none-any.whl (18 kB)
Installing collected packages: presto-types-parser
Successfully installed presto-types-parser-0.0.2
WARNING: You are using pip version 21.2.3; however, version 21.3.1 is available.
You should consider upgrading via the '/Users/amiorin/.pyenv/versions/3.9.7/envs/test/bin/python3.9 -m pip install --upgrade pip' command.
╭─amiorin@silicon ~/code/OpenMetadata/ingestion ‹main*›
╰─$ metadata ingest -c examples/workflows/trino.json
[2021-12-19 09:16:34,878] INFO     {metadata.ingestion.source.sql_source:361} - Parsing Data Models
[2021-12-19 09:16:38,124] INFO     {metadata.ingestion.source.sql_source:64} - Table Scanned: local_trino.call_center
[2021-12-19 09:16:38,124] ERROR    {metadata.ingestion.source.sql_source:148} - Table Description Error :
/Users/amiorin/.pyenv/versions/3.9.7/envs/test/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'char(16)' of column 'cc_call_center_id'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/test/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'varchar(50)' of column 'cc_name'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/test/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'varchar(50)' of column 'cc_class'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/test/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'char(20)' of column 'cc_hours'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/test/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'varchar(40)' of column 'cc_manager'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/test/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'char(50)' of column 'cc_mkt_class'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/test/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'varchar(100)' of column 'cc_mkt_desc'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/test/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'varchar(40)' of column 'cc_market_manager'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/test/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'varchar(50)' of column 'cc_division_name'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/test/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'char(50)' of column 'cc_company_name'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/test/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'char(10)' of column 'cc_street_number'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/test/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'varchar(60)' of column 'cc_street_name'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/test/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'char(15)' of column 'cc_street_type'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/test/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'char(10)' of column 'cc_suite_number'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/test/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'varchar(60)' of column 'cc_city'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/test/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'varchar(30)' of column 'cc_county'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/test/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'char(2)' of column 'cc_state'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/test/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'char(10)' of column 'cc_zip'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/test/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'varchar(20)' of column 'cc_country'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/test/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'decimal(5,2)' of column 'cc_gmt_offset'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
/Users/amiorin/.pyenv/versions/3.9.7/envs/test/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'decimal(5,2)' of column 'cc_tax_percentage'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))
[2021-12-19 09:16:41,279] WARNING  {metadata.ingestion.source.sql_source:534} - Unknown type NULL mapped to VARCHAR: cc_call_center_id
[2021-12-19 09:16:41,280] WARNING  {metadata.ingestion.source.sql_source:534} - Unknown type NULL mapped to VARCHAR: cc_name
[2021-12-19 09:16:41,280] WARNING  {metadata.ingestion.source.sql_source:534} - Unknown type NULL mapped to VARCHAR: cc_class
[2021-12-19 09:16:41,280] WARNING  {metadata.ingestion.source.sql_source:534} - Unknown type NULL mapped to VARCHAR: cc_hours
[2021-12-19 09:16:41,281] WARNING  {metadata.ingestion.source.sql_source:534} - Unknown type NULL mapped to VARCHAR: cc_manager
[2021-12-19 09:16:41,281] WARNING  {metadata.ingestion.source.sql_source:534} - Unknown type NULL mapped to VARCHAR: cc_mkt_class
[2021-12-19 09:16:41,282] WARNING  {metadata.ingestion.source.sql_source:534} - Unknown type NULL mapped to VARCHAR: cc_mkt_desc
[2021-12-19 09:16:41,282] WARNING  {metadata.ingestion.source.sql_source:534} - Unknown type NULL mapped to VARCHAR: cc_market_manager
[2021-12-19 09:16:41,283] WARNING  {metadata.ingestion.source.sql_source:534} - Unknown type NULL mapped to VARCHAR: cc_division_name
[2021-12-19 09:16:41,283] WARNING  {metadata.ingestion.source.sql_source:534} - Unknown type NULL mapped to VARCHAR: cc_company_name
[2021-12-19 09:16:41,283] WARNING  {metadata.ingestion.source.sql_source:534} - Unknown type NULL mapped to VARCHAR: cc_street_number
[2021-12-19 09:16:41,283] WARNING  {metadata.ingestion.source.sql_source:534} - Unknown type NULL mapped to VARCHAR: cc_street_name
[2021-12-19 09:16:41,284] WARNING  {metadata.ingestion.source.sql_source:534} - Unknown type NULL mapped to VARCHAR: cc_street_type
[2021-12-19 09:16:41,284] WARNING  {metadata.ingestion.source.sql_source:534} - Unknown type NULL mapped to VARCHAR: cc_suite_number
[2021-12-19 09:16:41,285] WARNING  {metadata.ingestion.source.sql_source:534} - Unknown type NULL mapped to VARCHAR: cc_city
[2021-12-19 09:16:41,285] WARNING  {metadata.ingestion.source.sql_source:534} - Unknown type NULL mapped to VARCHAR: cc_county
[2021-12-19 09:16:41,285] WARNING  {metadata.ingestion.source.sql_source:534} - Unknown type NULL mapped to VARCHAR: cc_state
[2021-12-19 09:16:41,286] WARNING  {metadata.ingestion.source.sql_source:534} - Unknown type NULL mapped to VARCHAR: cc_zip
[2021-12-19 09:16:41,286] WARNING  {metadata.ingestion.source.sql_source:534} - Unknown type NULL mapped to VARCHAR: cc_country
[2021-12-19 09:16:41,286] WARNING  {metadata.ingestion.source.sql_source:534} - Unknown type NULL mapped to VARCHAR: cc_gmt_offset
[2021-12-19 09:16:41,287] WARNING  {metadata.ingestion.source.sql_source:534} - Unknown type NULL mapped to VARCHAR: cc_tax_percentage

╰─$ pip list
Package                      Version    Location
---------------------------- ---------- ----------------------------------------------
beautifulsoup4               4.10.0
cachetools                   4.2.4
certifi                      2021.10.8
cffi                         1.15.0
charset-normalizer           2.0.9
click                        8.0.3
commonregex                  1.5.4
cryptography                 36.0.1
dnspython                    2.1.0
ecdsa                        0.17.0
email-validator              1.1.3
future                       0.18.2
google                       3.0.0
google-auth                  2.3.3
idna                         2.10
mypy-extensions              0.4.3
openmetadata-ingestion       0.5.0.dev0 /Users/amiorin/code/OpenMetadata/ingestion/src
openmetadata-sqlalchemy-hive 0.2.0
pip                          21.2.3
presto-types-parser          0.0.2
pure-sasl                    0.6.2
pyasn1                       0.4.8
pyasn1-modules               0.2.8
pycparser                    2.21
pydantic                     1.8.2
PyHive                       0.6.4
python-dateutil              2.8.2
python-jose                  3.3.0
PyYAML                       6.0
requests                     2.26.0
rsa                          4.8
sasl                         0.3.1
setuptools                   57.4.0
six                          1.16.0
soupsieve                    2.3.1
sql-metadata                 2.0.0
SQLAlchemy                   1.4.28
sqlparse                     0.4.2
thrift                       0.13.0
thrift-sasl                  0.4.3
typing_extensions            4.0.1
typing-inspect               0.7.1
urllib3                      1.26.7
wheel                        0.36.2
WARNING: You are using pip version 21.2.3; however, version 21.3.1 is available.
You should consider upgrading via the '/Users/amiorin/.pyenv/versions/3.9.7/envs/test/bin/python3.9 -m pip install --upgrade pip' command.

@amiorin I am confused. Why are you installing hive connector and running trino workflow? and is it hive or presto connector thats causing the issues?

(env)                                --   --   (main  origin)     
~/Code/OpenMetadata/ingestion • pip install -e '.[trino, hive]'                                                                                8:39  harsha@Sriharshas-MacBook-Pro
Obtaining file:///Users/harsha/Code/OpenMetadata/ingestion
  Preparing metadata (setup.py) ... done
Requirement already satisfied: google>=3.0.0 in ./env/lib/python3.9/site-packages (from openmetadata-ingestion==0.5.0.dev0) (3.0.0)
Requirement already satisfied: sql-metadata~=2.0.0 in ./env/lib/python3.9/site-packages (from openmetadata-ingestion==0.5.0.dev0) (2.0.0)
Requirement already satisfied: wheel~=0.36.2 in ./env/lib/python3.9/site-packages (from openmetadata-ingestion==0.5.0.dev0) (0.36.2)
Requirement already satisfied: cryptography in ./env/lib/python3.9/site-packages (from openmetadata-ingestion==0.5.0.dev0) (36.0.1)
Requirement already satisfied: python-dateutil>=2.8.1 in ./env/lib/python3.9/site-packages (from openmetadata-ingestion==0.5.0.dev0) (2.8.2)
Requirement already satisfied: requests~=2.26 in ./env/lib/python3.9/site-packages (from openmetadata-ingestion==0.5.0.dev0) (2.26.0)
Requirement already satisfied: commonregex in ./env/lib/python3.9/site-packages (from openmetadata-ingestion==0.5.0.dev0) (1.5.4)
Requirement already satisfied: idna<3,>=2.5 in ./env/lib/python3.9/site-packages (from openmetadata-ingestion==0.5.0.dev0) (2.10)
Requirement already satisfied: pydantic>=1.7.4 in ./env/lib/python3.9/site-packages (from openmetadata-ingestion==0.5.0.dev0) (1.8.2)
Requirement already satisfied: mypy_extensions>=0.4.3 in ./env/lib/python3.9/site-packages (from openmetadata-ingestion==0.5.0.dev0) (0.4.3)
Requirement already satisfied: sqlalchemy>=1.3.24 in ./env/lib/python3.9/site-packages (from openmetadata-ingestion==0.5.0.dev0) (1.4.28)
Requirement already satisfied: email-validator>=1.0.3 in ./env/lib/python3.9/site-packages (from openmetadata-ingestion==0.5.0.dev0) (1.1.3)
Requirement already satisfied: typing_extensions>=3.7.4 in ./env/lib/python3.9/site-packages (from openmetadata-ingestion==0.5.0.dev0) (4.0.1)
Requirement already satisfied: google-auth>=1.33.0 in ./env/lib/python3.9/site-packages (from openmetadata-ingestion==0.5.0.dev0) (2.3.3)
Requirement already satisfied: python-jose==3.3.0 in ./env/lib/python3.9/site-packages (from openmetadata-ingestion==0.5.0.dev0) (3.3.0)
Requirement already satisfied: click>=7.1.1 in ./env/lib/python3.9/site-packages (from openmetadata-ingestion==0.5.0.dev0) (8.0.3)
Requirement already satisfied: PyYAML in ./env/lib/python3.9/site-packages (from openmetadata-ingestion==0.5.0.dev0) (6.0)
Requirement already satisfied: typing-inspect in ./env/lib/python3.9/site-packages (from openmetadata-ingestion==0.5.0.dev0) (0.7.1)
Requirement already satisfied: sqlalchemy-trino in ./env/lib/python3.9/site-packages (from openmetadata-ingestion==0.5.0.dev0) (0.4.1)
Collecting sasl==0.3.1
  Using cached sasl-0.3.1.tar.gz (44 kB)

metadata ingest -c ./examples/workflows/trino.json                                                                 8:41  harsha@Sriharshas-MacBook-Pro
/Users/harsha/Code/OpenMetadata/ingestion/env/lib/python3.9/site-packages/sqlalchemy_trino/dialect.py:259: SAWarning: Dialect trino:rest will not make use of SQL compilation caching as it does not set the 'supports_statement_cache' attribute to ``True``.  This can have significant performance implications including some performance degradations in comparison to prior SQLAlchemy versions.  Dialect maintainers should seek to set this attribute to True after appropriate development and testing for SQLAlchemy 1.4 caching support.   Alternatively, this attribute may be set to False which will disable this warning. (Background on this error at: https://sqlalche.me/e/14/cprf)
  res = connection.execute(sql.text(query)).scalar()
[2021-12-19 08:41:07,628] INFO     {metadata.ingestion.source.sql_source:361} - Parsing Data Models
[2021-12-19 08:41:07,779] INFO     {metadata.ingestion.source.sql_source:64} - Table Scanned: local_trino.call_center
[2021-12-19 08:41:08,084] INFO     {metadata.ingestion.source.sql_source:218} - select * from tiny.call_center limit 50
[2021-12-19 08:41:08,234] INFO     {metadata.ingestion.sink.metadata_rest:208} - Successfully ingested table tiny.call_center
[2021-12-19 08:41:08,235] INFO     {metadata.ingestion.source.sql_source:64} - Table Scanned: local_trino.catalog_page
[2021-12-19 08:41:08,523] INFO     {metadata.ingestion.source.sql_source:218} - select * from tiny.catalog_page limit 50
[2021-12-19 08:41:08,709] INFO     {metadata.ingestion.sink.metadata_rest:208} - Successfully ingested table tiny.catalog_page
[2021-12-19 08:41:08,709] INFO     {metadata.ingestion.source.sql_source:64} - Table Scanned: local_trino.catalog_returns
[2021-12-19 08:41:09,018] INFO     {metadata.ingestion.source.sql_source:218} - select * from tiny.catalog_returns limit 50

@amiorin your last error happens if you install hive and try to run trino connector. We do have Trino connector
you can just install
pip install -e '.[trino]'
and it works
you can also have combination of
pip install -e '.[hive, trino, presto]'
and these connectors will work. I don't think is a bug.

I would consider it a bug. Both PyHive and sqlalchemy-trino were installed in my venv. I remove accidentaly sqlalchemy-trino.
Instead of getting
sqlalchemy.exc.NoSuchModuleError: Can't load plugin: sqlalchemy.dialects:trino

I got
[2021-12-19 18:36:54,665] INFO     {metadata.ingestion.source.sql_source:361} - Parsing Data Models
[2021-12-19 18:36:57,887] INFO     {metadata.ingestion.source.sql_source:64} - Table Scanned: local_trino.call_center
[2021-12-19 18:36:57,887] ERROR    {metadata.ingestion.source.sql_source:148} - Table Description Error :
/Users/amiorin/.pyenv/versions/3.9.7/envs/test2/lib/python3.9/site-packages/pyhive/sqlalchemy_presto.py:151: SAWarning: Did not recognize type 'char(16)' of column 'cc_call_center_id'
  util.warn("Did not recognize type '%s' of column '%s'" % (row.Type, row.Column))

And this can make our users think that there is a bug in our code instead of a misconfiguration. I could change the code so that it checks that sqlalchemy-trino is installed instead of PyHive uninstalled so that we leverage the fact that if both PyHive and sqlalchemy-trino are installed, sqlalchemy uses sqlalchemy-trino that is the version we support.
WDYT?

@amiorin That makes sense.


Improve client builder and options

This PR addresses a few last things:

[X] ClientBuilder ctor now accepts Options, not a naked array
[X] Adds Options:setDsn
[X] Removes the magic method ClientBuilder::__call that forwarded calls to the options
[X] Adds return types to ClientBuilderInterface

Last thing: should we make Options `final?

So, for the question related to making the Options class final :+1: from me. For the other points I see no real benefit in most of the changes. The one that concerns me most is that to instantiate the builder you now have to first instantiate the Options class and you lose a lot of simplicity for no real benefit since that class has to be created from an array of params anyway. We can provide a createFromOptions method if you really think that configuring the client options outside the builder is something really needed but I want to absolutely avoid over-complicating the builder instantiation

I can see your points. If we want to reduce the impact, we may reduce this PR to:

making Options final
add a ClientBuilder::createFromOptions()
Add a way to pass the dsn to Options at construction apart from the naked array, something like Options::createWithDsn().

@HazAT @ste93cry WDYT?

Both the first and second point of the list are fine. The last point makes me wonder if we really need it. What's the improvement of having a parameter dedicated to just the DSN instead of passing it directly together with all the other options?

The only bonus for the third point is that I'm forced to pass it with an array, always, contrary to all other options with have a setter instead.

contrary to all other options with have a setter instead.

This is because you can't change the DSN after creating a client since the transport won't be updated to reflect the new value. The only downside I see in making this change is that you're treating the DSN option differently from the others

I've reverted the setDns commit, and applied @HazAT suggestion from https://github.com/getsentry/sentry-php/pull/728#discussion_r242126295. Apart from that, IMHO we should leave the return types in the ClientBuilderInterface, since it's better that nothing, and @return $this is documented on each method to underline that is a fluent interface.

In that regard, The PHPDoc says @return $this, so the IDE should pick that up to infer the actual subtype of the builder in use.
Yes, we don't have contravariance in PHP so it's not solvable at the language level, but at the IDE level it seems fine to me.

Yes, we don't have contravariance in PHP so it's not solvable at the language level, but at the IDE level it seems fine to me.

Yea probably IDEs will be able to work if they give priority to the annotation instead of the typehint, but as you said yourself this problem is in the PHP language itself and we should not try to workaround it using a docblock since it's misleading and doesn't respect the real code

I've addressed or answered every comment from the review.

we should not try to workaround it using a docblock since it's misleading and doesn't respect the real code

I disagree on this. PHPDoc here helps to narrow the type that  can be inferred from the code alone, in the same way we document arrays as @var string[]. PHPStan and other static analysis tools like IDEs do give priority to PHPDoc over code for this exact reason; so in this way we have the stricter language barrier, and the right documentation with the help of PHPDoc.

I hope that with this example the reasons why I am against this change are clearer. Let's take as an example the following code:
interface HelloWorldInterface
{
    /**
     * @return $this
     */
    public function foo(): self;
}

class HelloWorld implements HelloWorldInterface
{
    /**
     * {@inheritdoc}
     */
    public function foo(): HelloWorldInterface
    {
        echo 'foo method called' . PHP_EOL;

        return $this;
    }

    public function bar(): self
    {
        echo 'bar method called' . PHP_EOL;

        return $this;
    }
}

(new HelloWorld())->foo()->bar()->foo();

This is perfectly fine and runs on PHP, however PHPStan reports an error. Infact the only reason the build is green is because you're ignoring the error in the phpstan.neon config file, so you are actually bypassing a language limitation using a docblock that doesn't match the real return typehint and its meaning by tricking the configuration of the static checker.

This seems a bug to me: I've moved @return $this directly on the implementation and PHPStorm stopped complaining. As per PHPStan, is a tracked bug: https://github.com/phpstan/phpstan/issues/1648
So I would like to avoid changing the correctness of the code basing our choiches on tools that will be fixed in the future. Unfortunately PSR-5 is still under discussion, so it's not reliable for now.


added transaction integration specification

Adding an integration specification around generating transaction through to the childchain API. supposedly generalized enough so that implementation could be done agnostic to the programming language

Lower case for Base16 encoding is required. This is worth noting.

nice catch ! @paulperegud I will add that in

@Pongch The wording used suggests that we need to convert to lower case before encoding. Whole thing is about Base.encode16(input, case: :lower). Sorry about possible confusion.

ah I see, I think this might come down to specific library use unique to the language itself. ie. this is the base16 library used in elixir is https://hexdocs.pm/elixir/Base.html which can strictly enforce case, while in JavaScript the lib isnt explicit about capitalized case :  https://www.npmjs.com/package/base-x

JS users can achieve needed effect by converting case to lower on the
string after encoding it to base16.
On Mon, Sep 24, 2018 at 3:15 PM Pongch<EMAIL_ADDRESS>wrote:

ah I see, I think this might come down to specific library use unique to
the language itself. ie. this is the base16 library used in elixir is
https://hexdocs.pm/elixir/Base.html which can strictly enforce case,
while in JavaScript the lib isnt explicit about capitalized case :
https://www.npmjs.com/package/base-x
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/omisego/elixir-omg/pull/186#issuecomment-423970252,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAPFevGSRaPPIw8cvL7USoLGVNxlR6Fpks5ueNrggaJpZM4W2HRg
.

--
Best regards,
Paul Peregud
+48602112091

Thing is - elixir-omg will reject transaction if it will spot capital case
letter in the encoded text. We just have to reword this part in doc to make
it clear.
On Mon, Sep 24, 2018 at 4:32 PM Paul Peregud<EMAIL_ADDRESS>wrote:

JS users can achieve needed effect by converting case to lower on the
string after encoding it to base16.
On Mon, Sep 24, 2018 at 3:15 PM Pongch<EMAIL_ADDRESS>wrote:

ah I see, I think this might come down to specific library use unique to
the language itself. ie. this is the base16 library used in elixir is
https://hexdocs.pm/elixir/Base.html which can strictly enforce case,
while in JavaScript the lib isnt explicit about capitalized case :
https://www.npmjs.com/package/base-x
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/omisego/elixir-omg/pull/186#issuecomment-423970252,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAPFevGSRaPPIw8cvL7USoLGVNxlR6Fpks5ueNrggaJpZM4W2HRg
.

--
Best regards,
Paul Peregud
+48602112091

--
Best regards,
Paul Peregud
+48602112091


New update

As requested in KSP-CKAN/NetKAN/pull/2173

desc has to be deleted?

Passes local, this is failing because older versions of ModuleManager are no longer available.


Serial debugging failing to start

This issue was found on a physical ASA 5505 running version 9.1(6).
When enabling gdbserver on the serial interface in asafw, the gdb script generated by asadbg hangs at target extended-remote [serial port].

Turning on gdb's remote debugging shows that gdbserver continually prints this string, preventing gdb from attaching:

It looks like a fix for this would be modifying the inittab file to run /tmp/start_cmd on a different tty. This will still cause gdbserver to attach to the serial interface, but will result in the error message being printed on the other tty as opposed to over the serial interface. Manually making this change on my end results in asadbg hanging (since it's waiting for the "Remote debugging over /dev/ttyS0" string) but eventually connecting over USB:

I'm willing to write the code to implement this change, but I'll hold off on implementing and making a pull request since this is a little more involved than my previous two issues.

Thanks for this.
Good finding about modifying /etc/inittab. So you effectively change: ttyS0::once:/tmp/run_cmd into ttyS1::once:/tmp/run_cmd? Automation could quite easily be tested by adding code to unpack_repack.sh in the custom function and invoking unpack_repack_bin.sh -c .... Once this works we could add a special case in the enable_gdb function for the firmware we know require that (e.g. "asa91"). Afaict most of the 9.1.x do require that.
I think if we can find a way to have gdb not hang at all, it will be even better. Not sure if we can tell gdb to not wait for the "Remote debugging using /dev/ttyS0" string. Tbh I didn't even know it worked like that.
What is interesting is that for asa924 for instance, we have this fix (note the comment which is not on asa91x):
# run on the console specified on the kernel start line
::once:/tmp/run_cmd

I am wondering if just changing to this would solve the gdb hanging problem.
Yes, I agree it involves a bit more changes that your previous PR but at the same time is a good exercise to be able to modify the scripts (and not too complex in this case imho) so if that is something that you are interested in, feel free to do so.
Note: In the past, we would use --bin-with-asa-to-inject to e.g. inject the /asa folder from 9.1.x into another firmware such as asa92*.bin but your way is a lot cleaner and does not change the firmware container.

Oh right thanks for explaining that after your fix, the hang is on the asadbg side, not gdb side.  The reason there is a hang is because we timeout after 3 minutes in case we don't detect the "Remote debugging using /dev/ttyS0" string which indeed we don't detect following your fix. We should be able to work around that but not sure how yet either.
Good idea about the characters gdbserver may send over serial. I guess we could hexdump what we receive in comm.py instead of just printing ascii characters, to see if there is anything interesting.
In case there isn't any character send, it may be possible to check from comm.py if there is a listening gdbserver on the other end by sending special characters. So in case we read 0 bytes, we could send this special series of bytes from comm.py and check we get the right reply from the other end (gdbserver).

Definitely worth looking into. I'll get started on implementing the change for unpack_repack_bin.sh this weekend, and open an issue for the comm.py stuff over on the asadbg repo after I do a little more research into the gdbserver stuff. Thanks for your insight :)


fix: add hash when redirecting to target language

Summary

fix #154
Details

copilot:walkthrough
Related Issue

Checklist

[ ] I have added changeset via pnpm run change.
[ ] I have updated the documentation.
[ ] I have added tests to cover my changes.

Closed due to outdated


"Password" setting type

Sometimes you might be streaming or otherwise showing your setting tab and you don't want the value of a setting shown... not that that's ever happened :eyes:

Added in 3.6.4634-dev


Using .catch() for error handling instead of try catch

Problem
One of the things that upsets me when I face an error (besides the error itself) is how the server crashes and I should restart it every time.
One thing that  also that doesn't make me really happy is how I should the try catch blocks, it's annoying makes writing code harder that it should be since the data inside the try block is not accessible outside of it.
Suggested solution
What I want is a way to handle errors in a way that prevent errors from crashing the server and without the need of try catch blocks, thus I suggest adding .catch() for prisma queries.
Alternatives
A nice alternative is making prisma client handle the errors automatically without crashing the server.
Additional context
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/catch

It appears that this issue was labeled as a bug
This issue was opened because whenever I  try to use  .catch()  with FindFirst function, it send expecting try catch error.
FindMany function worked with .catch()  worked fine, although I didn't test how it does react to an error.

Any updates on this ?

Any updates on this?


Pass an unhandled exception to at_exit block as second argument

Follow up #1921
It is better in some ways:

it does not need a new exception like SystemExit.
it does not break compatibility in most cases because block fill up lacking arguments. For example, this PR does not fix existing tests for at_exit.

I think I like this. Could you update the documentation a bit on when to expect something for that argument?


Snapshot: Create a voting strategy that use voters BrightID score

Here is the challenge:
Create a voting strategy that use voters BrightID score
We are looking for someone to create a Snapshot voting strategy that can take the voters score from BrightID and use it as a voting power. This issue will be funded on Gitcoin with a value of $200.
Context
On Snapshot, the strategies are JavaScript functions used to calculate the results of a proposal. The function take as input the list of voters and return a score for each of them. A proposal can use one or multiple strategies, the most common one is "erc20-balance-of", this strategy count the balances of the voters for a specific ERC20 token. Within a strategy you can query data from smart contract, a subgraph or an external API and build your own logic on how results are calculated.
You can find all the available strategies here https://snapshot.page/#/strategies
Learn more https://docs.snapshot.page/strategies
Got questions? Join our Discord on https://discord.snapshot.org/

@ananta813 Hi, i can't see your submission, anything i'm missing?

I love the idea for this BrightID enabled Snapshot strategy.
I will personally offer an additional .1 ETH bounty to whoever earns the primary bounty from @bonustrack for completing this task.

Many thanks for the support @UBIpromoter !

We will make another bounty more specific for the integration.


feature request: Add Color merge functionality

I have two color objects that I'd like to "merge" together. A quick example would be:
func foo(a color.Color, b color.Color) {
	a.Merge(b).Println("hello world")
}

func main() {
	color1 := color.New(color.BgRed)
	color2 := color.New(color.FgGreen)

	modifier1 := color.New(color.Underline)
	modifier2 := color.New(color.Bold)

	foo(color1, modifier1)
	foo(color1, modifier2)
	foo(color2, modifier1)
	foo(color2, modifier2)
}

This could follow the same semantics as Color.Add in terms of overwriting non-compatible attributes.
Alternatively, exposing the current attributes of a color object would allow me to do a.Add(b.Attributes...) to achieve the same effect.
If this seems reasonable I can submit a PR

Probably as simple as append(a.Attributes, b.Attributes).

Yeah, internally I think this would be as simple as this (based on the current Add implementation)
func (c *Color) Merge(others ...*Color) *Color {
	for other := range others {
		c.params = append(c.params, other.params...)
	}
	return c
}

@apexskier If you don't mind I'll submit a PR.

Go for it

@apexskier Are you thinking for it to modify the original Color, or return a new one?

It could also be like this:
a := color.New(color.FgRed)
b := color.New(color.Bold, color.BgGreen)
c := color.New(color.Italic)
c := color.Merge(a, b, c /* ... */)        // returns a new color without modifying a, b, or c

But that isn't like the other functions because most of them are methods with a receiver of type *Color.
@apexskier What do you think?

@apexskier I've got it done now.  Just need to confirm which path to go, modify receiver or return a new Color object.  I personally like the idea better of returning a new color object with colors.Merge.

I like the more immutable pattern you're suggesting. That'll work better for my use case.

Closing this based on the comment here: https://github.com/fatih/color/pull/129#issuecomment-770465089


Allow search path to be extended with environment variable

Hi @aep,
What are your thoughts on allowing one the ability to add a path to the search paths HashSet with an environment variable? This could allow users to specify the location of their modules if they are not located in standard directories when running zz:
$ ZZ_MODULES_DIR=./path/to/modules zz build

I think something like:
if let Ok(modules_dir) = std::env::var("ZZ_MODULES_DIR") {
    searchpaths.insert(std::path::Path::new(&modules_dir).to_path_buf());
}

Yes this is a good idea, thanks for the suggestion

Great! I issued an initial implementation in a PR (#38)


Spring Cloud Vault incompatible with spring-cloud version 2020.0.2 (method createSslConfiguration does not exist)

Describe the bug
I create my project on start.spring.io, adding only vault.
The generated pom has <spring-cloud.version>2020.0.2</spring-cloud.version> (notice the .2) for spring-cloud-dependencies
and uses the spring-cloud-starter-vault-config dependency.
On mvn clean install the test fails to start the application context with
An attempt was made to call a method that does not exist. The attempt was made from the following location:

  org.springframework.cloud.vault.config.VaultConfiguration.createSslConfiguration(VaultConfiguration.java:101)

The following method did not exist:

  'void org.springframework.vault.support.SslConfiguration.<init>(org.springframework.vault.support.SslConfiguration$KeyStoreConfiguration, org.springframework.vault.support.SslConfiguration$KeyStoreConfiguration, java.util.List, java.util.List)'

The method's class, org.springframework.vault.support.SslConfiguration, is available from the following locations:

  jar:file:/Users/hlmwiddershoven/.m2/repository/org/springframework/vault/spring-vault-core/2.3.0/spring-vault-core-2.3.0.jar!/org/springframework/vault/support/SslConfiguration.class

The class hierarchy was loaded from the following locations:

  org.springframework.vault.support.SslConfiguration: file:/Users/hlmwiddershoven/.m2/repository/org/springframework/vault/spring-vault-core/2.3.0/spring-vault-core-2.3.0.jar

If I change the spring cloud version to 2020.0.1 the application starts, and even works if I set @SpringBootTest(properties = "spring.cloud.vault.token=123" ) on the generated test class.
Sample
Go to start.spring.io and create an app with just vault, and check that the spring cloud version is 2020.0.2.

Spring Cloud 2020.0.2, specifically Spring Cloud Config, pulls in an outdated version of Spring Vault. The SslConfiguration constructor was introduced with Spring Vault 2.3.2.
See https://github.com/spring-cloud/spring-cloud-config/commit/20be22eced2700e257c3ebf1ae65a300570db824 for reference.


"distanceToMesh()" not working properly when meshes are both far away

Hi,
I'm trying to compute the distance between two meshes, but I found that this issue occurs even with the two spheres. In this case, there is no issue:
from vedo import Sphere, show
s1 = Sphere(r=12896.0)
s2 = Sphere(pos = (80146.7021484375, 21942.59130859375, 15899.5),r = 12840.5, c='white', alpha=0.4)

s1.distanceToMesh(s2, signed=True, negate=False)

s1.addScalarBar(title='Signed\nDistance')

print(s1.pointdata["Distance"])
show(s1, s2, __doc__ , axes=11).close()

As soon as I change the origin of the first sphere, s1.pointdata['Distance'] is an empty list, no distances are computed:
from vedo import Sphere, show
s1 = Sphere(pos = (29744.0, 21995.87841796875, 15836.0), r=12896.0)
s2 = Sphere(pos = (80146.7021484375, 21942.59130859375, 15899.5),r = 12840.5, c='white', alpha=0.4)

s1.distanceToMesh(s2, signed=True, negate=False)

s1.addScalarBar(title='Signed\nDistance')

print(s1.pointdata["Distance"])
show(s1, s2, __doc__ , axes=11).close()

What could be the issue here?

Sorry for the late reply.

Cannot find command 'git'

indeed your anaconda might not have git installed...

somewhat of a similar issue occurring with Box shapes/meshes

the above fix also fixes the box example


setupSnapshot() Causing AWS Failure

New Issue Checklist

[X] Updated fastlane to the latest version
[X] I have read the Contribution Guidelines

Issue Description
This is an issue I'm running into while setting up XCode 7 UI Tests to support the latest version of Snapshot.  I have been following the UI Tests Quick Start guide, but have run into a problem running my tests with Snapshot from XCode 7.
Complete Contents of My Test Swift File:
import XCTest

class CFPlusUITests_Snapshot: XCTestCase {

    override func setUp() {
        super.setUp()

        // Put setup code here. This method is called before the invocation of each test method in the class.

        // In UI tests it is usually best to stop immediately when a failure occurs.
        continueAfterFailure = false
        // UI tests must launch the application that they test. Doing this in setup will make sure it happens for each test method.
        let app = XCUIApplication()
        setupSnapshot(app)
        app.launch()

        // In UI tests it’s important to set the initial state - such as interface orientation - required for your tests before they run. The setUp method is a good place to do this.
    }

    override func tearDown() {
        // Put teardown code here. This method is called after the invocation of each test method in the class.
        super.tearDown()
    }

    func testExample() {

        let app = XCUIApplication()

        sleep(10)

        // Just open the app, select the first fair, and then wait for the dashboard to appear
        let firstChild = app.tables.childrenMatchingType(.Cell).elementBoundByIndex(0);
        if (firstChild.exists) {
            NSLog("First child does exist!");
            firstChild.tap()
        } else {
            NSLog("First child does not exist.");
        }

        sleep(5)
    }
}

Running these tests from XCode using Command-U successfully launches the app, but I receive the following in-app error when attempting to connect to our AWS S3 bucket:

If, however, I simply comment out the setupSnapshot(app) line from the above code the app runs successfully.
I am very new to using UITests, so I am open to any advice for the best way to debug this issue.  So far I have not been able to get any additional information about why this might be occurring.

Hi @cdunkel. Thanks for reaching out about this. Are you using any type of special transport security with your S3 bucket or just direct SSL? The reason I ask is because this error suggests that there is some sort of request signature that's being altered from it's expected value. Thanks!

@TKBurner I'm not doing anything special.  All of the transport security is wrapped up in the AWS iOS SDK that I'm using, which has fairly terrible documentation, so I'm not sure exactly what they're using under the covers.  I'm simply providing an access key and a secret key.

@cdunkel Thanks for following up. I'm not too familiar with AWS iOS SDK either. This seems like it's something related to your integration. Check out this page for some info on that specific error:
http://docs.aws.amazon.com/general/latest/gr/signature-v4-troubleshooting.html and this issue in their github repo https://github.com/aws/aws-sdk-js/issues/86

@TKBurner Thanks for the response!  I had actually come across the doc you referenced previously, but not the issue.  That could be helpful!  I will spend some time today or tomorrow and see if I can get some more logs from my app that might shed some light onto why the setupSnapshot(app) call might be causing issues.

@cdunkel Awesome! Let me know how it goes 👍

Update: My captured app logs didn't really give me any more information on this.  I have posted an issue to the AWS iOS SDK repo, too, to get some advice from that side of things.
https://github.com/aws/aws-sdk-ios/issues/381

I figured it out!
Part of what the setupSnapshot(app) function does is to set the language for the app.  It does this using the language.txt file located in ~/Library/Caches/tools.fastlane.
However, if you have not run snapshot from the console yet, then the language.txt file does not exist and, rather than throwing an error, it sets the simulator device language to "".  This doesn't seem to affect the ability for the app to run in the simulator, but does cause requests to AWS S3 to fail.
So the "solution" was to run snapshot in the console first, but it seems like the setupSnapshot(app) function should really fail out if that language file is not found, or at least fail in a way that does not adversely affect the operation of the app.
My Swift isn't great, but it appears that deviceLanguage = try NSString(contentsOfFile: path, encoding: NSUTF8StringEncoding).stringByTrimmingCharactersInSet(trimCharacterSet) as String is happy to return nil if the file at path doesn't exist.  What should probably happen is that returned var should be checked before setting the language.
I can create a PR for that change, if desired.

@cdunkel Thanks for following up. This is expected behavior. Did you run snapshot init in this folder before you ran the lane? That seems to be where things broke.
Thanks!

@TKBurner I'm actually using Snapshot independently of a lane, since most of my iOS build structure was implemented before Fastlane combined all the tools.  I did run snapshot init in my iOS project folder before attempting to link Snapshot up with my tests, but the language.txt file I referenced only seems to be created on actually running snapshot from the console (or through a lane, I assume).  So my real mistake was attempting to run by UITest target through XCode before running snapshot through the console.
It still seems odd to me that if the language.txt file doesn't exist - for whatever reason - the setLanguage() function in SnapshotHelper.swift doesn't hit the "Couldn't detect/set language..." block.

Thanks @cdunkel! That makes sense. I'm going to let the team know about this potential issue so we can address it going forward. Thanks!

@TKBurner Awesome, thanks!  And thank you again for all the help!


RISC-V CI for memcached

I have seen the status of memcached in the RISC-V software ecosystem sheet as "In-progress" for RISC-V. I am assuming that this is not yet fully supported for RISC-V. Also the CI for Memcached is running on github actions as I have seen in the commit status.
I would like to inform you that there is a RISC-V CI platform called Cloud-V (created in collaboration with RISC-V Labs subgroup) which provides emulated and physical hardware for RISC-V software development. You can apply for access at this link.

I actually bought a risc-v board and will be setting it up sometime soon.
I'm pretty sure memcached actually works just fine on risc-v: the problem is these systems people are running the test suite on are so pathologically slow they are exposing timing bugs in the tests. I haven't seen an actual bug that needs to be fixed so far.

I can set up free CI for testing Memcached on various RISC-V boards or on QEMU user mode/QEMU system (which are faster than the boards because of faster host machines) using Cloud-V.
All I need is an access token to GitHub.


How to watch the root directory?

Is it possible to watch the root directory? I couldn't get this working:
var server = flo(__dirname, ...)

This should also work, or relative paths also work. Reopen if this is still a problem


Retry pypi.org requests on CI, or if an environment variable is set

Use PIPENV_MAX_RETRIES environment variable to determine how many times to retry requests to pypi.org. Default to 0 retries, unless running on CI, in which case default to 1 retry.
This was the suggested retry setup in the discussion on https://github.com/pypa/pipenv/issues/1110.

we discussed the CI check previously in the referenced issue (thanks for referencing that @greysteil) and after re-reading our previous discussion of this it seems like we just never got around to implementing it, this looks good to me

FYI I'm actually pretty sure pip sets this to 3, so maybe we should just always default to that?

From the pip help output:
--retries <retries>         Maximum number of retries each connection should attempt (default 5 times).

Happy to change to a default of 5 for CI if you want to match that behaviour (the pip default if --retries isn't passed is 0).

oh I see, that's a confusing bit of help text.  Lets just set it to one for now then, I assume you were hitting a bug and this fixed it? I think we should be pragmatic on this one and assume if it fixes the problem we can just go with the conservative option

👍 and yep, exactly - was tailing Dependabot's log and noticed it hit this on one dependency before succeeding in updating a subsequent one moments later.


Github Action to Auto-Assign Reviewers to Pull Requests

Added Github Action to auto-assign HarshCasper, iamrajiv, and atarax665 to pull requests

Have you read the Contributing Guidelines on Pull Requests?
Yes
Description
Added a Github Action to auto-assign reviewers to pull requests.
Checklist

[ ] I've read the contribution guidelines.
[ ] I've checked the issue list before deciding what to submit.
[ ] I've edited the README.md and link to my code.

Related Issues or Pull Requests
https://github.com/TesseractCoding/NeoAlgo/issues/1770

Looks like Github Actions may not have the necessary permissions when working from forked repositories. Receiving this error on the check: "Resource not accessible by integration". Please let me know how you would like me to proceed. @HarshCasper @atarax665


Adding a short list of known build integrations

Gulp, Webpack, Grunt...

https://github.com/yahoo/atomizer/issues/252


Release new version

Hey!
Thanks for supporting this gem. It's been a while and a few new emojis since last version release, do you care for a new one?
Thanks

I'm having problems with this one: 🌠


weight files

could you upload the weight files ?

Sorry for the delayed reply.
Now we the uploaded initial version of our code with pre-trained weights.
We are still working for Section 4 (evaluate with SIDD and DND) of README but I checked that other sections works well.
Please check the new version code and let me know if you have an additional question.
Sungmin


344 | Reverse String | Python

Reverse String
Type of Issue -

Adding New Code

Programming Language

Python

Self Check

Ask for issue assignment and wait to get assigned to it before making Pull Request.
Add your file in the proper folder
Clean Code and Documentation for better readability
Add Title and Description of the program in the file

:star2: Star it :fork_and_knife:Fork it :handshake: Contribute to it!
Discord server ✉ - https://discord.gg/CXksGdn
Happy Coding,

Please assign me question 344

I like to do it. Assign me

[x] Assigning to @JayeshSareen for issue creation.


net/netip: IP address bit manipulation functions

This seems like a potentially much larger topic than the "little things" of https://github.com/golang/go/issues/49298 so I've made a separate issue for it.
I have a trie data structure that works over net.IP with lots of bit twiddling. It works well, but of course I want to switch it over to netip.Addr, and benefit from performance there. However, there currently is no way of getting at its internal uint128 guts, or doing any arithmetic therein.
Seems like this would be a nice thing to add. I don't have a concrete proposal for how to do this yet (so this isn't a "proposal: " issue), but I thought I should still start the discussion.

CC @bradfitz @crawshaw @josharian @danderson @ianlancetaylor

As16 and AddrFrom16 are allocation-free.
Also, I plan to make a new package that unifies the inet.af/netaddr and net/netip with the missing pieces. That'll likewise rely on being able to do zero-alloc conversions back and forth.

allocation-free, but it still copies to the stack, right? Is SSA able to eliminate those somehow?

Still copies to the stack, but it's super cheap. And I'll send a CL soon that'll make it even cheaper. I think this API is fine.

API wise, I was hoping some of those uint128 accessors could somehow get exposed, so I could index bits and and, or, and xor limbs together without having to drop down to bytes or roll it all myself. But I could also understand if that's not desirable in general to do.

I hope we get that via #9455. In the interim, we should copy/paste the uint128 code around as needed. API is impossible to claw back, so for now we should be conservative.

Alright.
Hopefully the compiler can optimize:
Addr -> endian converstion -> array -> endian conversion -> myu128 -> arithmetic -> myuint128 -> endian conversion -> array -> endian conversion -> Addr
Into
Addr -> arithmetic -> Addr
That seems like an impressive feat though.

Let's see how it does.


Create Event screen needs some css widths adjusted within the responsive design

https://goo.gl/bKrstE
Once the display is small enough, the labels on the boxes respond appropriately.
There is a point however between ~1200 pixels wide and ~1000 pixels wide where the labels are covered by the data fields
The page to create campaigns seems to handle this much better: https://goo.gl/goZ24c

Working on this one.

@tonysurma
This is not applicable any more.
The only thing that's not nice is the long label Enforce volunteer limit on tasks and the moment the labels pop above the inputs.


Document more details about query execution

See especially https://github.com/prometheus/prometheus/issues/2564, where it was unclear what the resolution step parameter meant, and that/why queries have an evaluation step that's independent of underlying data.

Hi, @juliusv . Any progress on this issue?
In case of step is independent of underlying data, i wanna know:

how instant query result be aligned to step
how range query result be aligned to step

@chanjarster

Instant queries do not have a step, as they are evaluated at a single point in time (the current time, or a custom time, if it is supplied via the API).
See https://github.com/prometheus/prometheus/blob/a15a5a2ba095efff1012d735ecc7ecea4224ca7d/promql/engine.go#L795. interval represents the step in that evaluation loop. We start at the range's start timestamp, then increase the timestamp by interval on every iteration, and abort the loop when the timestamp becomes larger than the range's end timestamp.

@juliusv Thanks for quick reply. I still have something to confirm on range query, please tell me if I'm wrong:
For example, there is a range query startTs=0, endTs=16, interval=5
And there are data points: ts=1,v=1; ts=4,v=4; ts=7,v=7; ts=11,v=11; ts=15,v=15
Result will be: ts=0,v=1; ts=5,v=7; ts=15,v=15
Am I right?

why not is

@chanjarster At every evaluation step, PromQL selects the value of the sample that is the most recent before (or exactly at) the evaluation step timestamp, so I think the result would be:
ts=0,v=1; ts=5,v=4, ts=10,v=7, ts=15,v=15
Note that there will be always one returned sample for every increment of the interval (5), even if it would be the same repeated value from the same sample (if "real" samples are less frequent than the eval step). The exception is when there is an explicit staleness marker or the last sample for a given series is >5min before the evaluation timestamp (promql.LookbackDelta = 5 * time.Minute).

I can understand ts=5,v=4, ts=10,v=7, ts=15,v=15 , but I don't know why value=1 when ts=0 ,can you help me?

@princeliang Oh thanks for catching that, that was indeed just a mistake. An evaluation step never looks at "future" data, so for ts=0, the result would simply be empty instead of 1.

Just a quick ping, since querying is how people interact with Prometheus. @brian-brazil can you help with this?
I came here from the mentioned issue (https://github.com/prometheus/prometheus/issues/2564), and I still have no idea why Prometheus returned an empty list before I had changed the resolution field. Either I have no idea why it returned data after I changed that field.


Fixed bug in 'test_mesos.test_if_ucr_app_runs_in_new_pid_namespace'.

Previously, there was a race between the app_task starting and it
writing its output to the ps_output_file. We now wait up to 10 seconds
for this file to appear before giving up and throwing an exception.
High Level Description
What features does this change enable? What bugs does this change fix? High-Level description of how things changed.
Related Issues

DCOS_OSS-862 Vagrant integration test test_mesos.test_if_ucr_app_runs_in_new_pid_namespace is flaky.

Checklist for all PR's

[ ] Included a test which will fail if code is reverted but test is not. If there is no test please explain here: This is a fix to a test itself
[x] Read the DC/OS contributing guidelines
[x] Followed relevant code rules Rules for Packages and Systemd

Checklist for component/package updates:
If you are changing components or packages in DC/OS (e.g. you are bumping the sha or ref of anything underneath packages), then in addition to the above please also include:

[ ] Change log from the last version integrated (this should be a link to commits for easy verification and review): example
[ ] Test Results: [link to CI job test results for component]
[ ] Code Coverage (if available): [link to code coverage report]

@mesosphere-mergebot label [Ready For Review]

I'm having deja vu... how is this PR different than https://github.com/dcos/dcos/pull/1617 ?

Shouldn't we wait for the one on master then and then backport it? Why review it twice?

Ideally, yes, we would land it in master first, then backport it.
But a) sometimes the patch release has more urgency, so we end up landing
it there first.
b) any cherry-pick conflicts when backporting would require a nominal
review to make sure the backporting didn't screw anything up.
On Tue, Jun 6, 2017 at 3:35 PM, Karl Isenberg<EMAIL_ADDRESS>wrote:

Shouldn't we wait for the one on master then and then backport it? Why
review it twice?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/dcos/dcos/pull/1616#issuecomment-306636971, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AGKE0BzJ-9hG7ZjVYPFRiQLB7zDyJJ7hks5sBdRIgaJpZM4Nu7La
.


Update plant for dealing with float values

Value parsing in plant component throws an ValueError if values are given as floats. This commit changes int(value) to int(float(value)) to avoid this error.
The change is minimal and should not have any side effect

Hi @dominikandreas,
It seems you haven't yet signed a CLA. Please do so here.
Once you do that we will be able to review and accept this pull request.
Thanks!


Added handling of validateAuthority parameter

Linked with issue #15

Thanks for this fix. Will update npm module as soon as possible.


301971 remove all calls to logyard from logyard apps lgr

Updated the import path to non-relative path. This fix is required if we are migrating logyard-apps-lgr to use godep.

LGTM, assuming this went through go fmt.
Why is that commit from Alex being pulled in? That should probably not be there?

@Mook-as Yes, I ran it through go fmt. I need to merge to master, any objections?


Add CORS and HTTP status code test for ListReferenceBases

CORS support is required for servers to support JavaScript clients. Tests verifying the server's CORS support are in tests/unit/test_views.py, which tests other high-level HTTP related things (this test module is now poorly named, and should really be test_http.py or something; the hackathon isn't the time for this sort of refactor though).

Edit the tests/unit/test_views.py, and find the testCors function. Run the test using nosetests -vs tests/unit/test_views.py:TestFrontend.testCors.

Add a check to run the ListReferenceBases request. Remove the TODO.

Add a function testListReferenceBases that follows the pattern of testGetVariantSet to ensure we get the right HTTP responses. The ListRefernenceBases method is defined in the schemas, and is an oddball as it doesn't follow the conventions of other server endpoints.

Optional The requirement for CORS support is not documented anywhere in the protocol. Open a documentation issue on the schemas repo, requesting clarification on this point.

Looks like the list reference bases endpoint is already tested (self.sendReferenceBasesList) but these four endpoints are not:

reference sets search 2) read group sets search 3) call sets search 4) call sets get


Add Option to Prevent Outside Click Closing of the Modal

First off, thanks for this awesome package. There are some instances where I don't want the modal to auto-close when the user clicks outside of the modal.
An example is: the modal displays a form where a user enters a bunch of data and then accidentally clicks outside and loses everything. I can ensure they don't actually lose anything with localStorage or some global state management solution, but I could prevent all of that with a simple option that turns off the auto-close feature on outside click.
I do recognize that I can simply implement my own ModalWrapper, but making this a core option would probably benefit a lot of people and seems very easy to implement. If you agree that this would be beneficial to the community, I also wouldn't mind making a pull request to expedite things.

It occurred to me that this sort of behavior is already supported. In case anyone comes across this in the future, the ModalWrapper component accepts an onClose prop and will run that function anytime an outside click is detected. Passing an empty function disables it from auto closing. This is what I ended up doing:
<ModalWrapper onClose={ disableOutsideClickAutoClose ? () => {} : () => Reoverlay.hideModal() } />


Connection configuration

This was previously discussed in #61.
There are quite a few things in SQLite that need to be configured every time a connection is opened. (E.g. certain PRAGMAs, user-defined functions, collations, etc.)
The idea would be that you pass some configuration into your SqliteConnection instances, and every time the connection is opened, it would be configured appropriately.

For the PRAGMAs we could take the same route as System.Data.SQLite does it: use certain keywords in the ConnectionString. Looking at the ones from System.Data.SQLite one could go with:

Foreign Keys: True, False
Journal Mode: Delete, Truncate, Persist, Memory, Wal, Off
Recursive Triggers: True, False
Synchronous: Off, Normal, Full, Extra

Additional PRAGMA settings of interest could be:

Auto Vacuum: None, Full, Incremental
Automatic Index: True, False

For collations or user defined functions we would need an additional configuration object, but the information storage in this object will be tricky. One could add the same functions as defined in #14 and #19 inside this SqliteConnectionConfiguration class and use them to store the given information in lists which would be parsed later in the Open call of the collation.

I'm not a big fan of the things System.Data.Sqlite added to the connection string. In my mind, things in the connection string should be things you can't change after the connection is opened.
You ideas about SqliteConnectionConfiguration match mine.
var config = new SqliteConfiguration();
config.AddFunction("func" myDelegate);

var connection = new SqliteConnection(connectionString, config);

connection.Open(); // Automatically calls connection.AddFunction("func" myDelegate);

Okay, then the keywords mentioned above would be properties of the SqliteConnectionConfiguration.
Proposal would than be:
public class SqliteConnectionConfiguration
{
    public bool ForeignKeys {get; set;} = false; // default?
    public bool RecursiveTriggers {get; set;} = false; // default?
    public JournalMode JournalMode {get; set;} = JournalMode.Wal;// enum name? default?
    public SynchronousMode Synchronous {get; set;} = SynchronousMode.Full; // default?
    public AutoVacuumMode AutoVacuum {get; set;} = AutoVacuumMode.None; // default?
    public AutomaticIndex {get; set;} = true; // default?

    public void AddCollation(
        string name,
        Comparison<string> comparison);

    public void AddCollation<T>(
        string name,
        T state,
        Func<T, string, string, int> comparison);

    public virtual void AddFunction<TResult>(
        string name,
        Func<TResult> function);

    public virtual void AddFunction<T1..16, TResult>(
        string name,
        Func<T1..16, TResult> function);

    public virtual void AddFunction<TState, TResult>(
        string name,
        TState state,
        Func<TState, TResult> function);

    public virtual void AddFunction<TState, T1..15, TResult>(
        string name,
        TState state,
        Func<TState, T1..15, TResult> function);

    public virtual void AddAggregate<TContext>(
        string name,
        Func<TContext, TContext> func);

    public virtual void AddAggregate<TContext, T1..15>(
        string name,
        Func<TContext, T1..15, TContext> func);

    public virtual void AddAggregate<TContext>(
        string name,
        TContext seed,
        Func<TContext, TContext> func);

    public virtual void AddAggregate<TContext, T1..15>(
        string name,
        TContext seed,
        Func<TContext, T1..15, TContext> func);

    public virtual void AddAggregate<TContext, TResult>(
        string name,
        TContext seed,
        Func<TContext, TContext> func,
        Func<TContext, TResult> resultSelector);

    public virtual void AddAggregate<TContext, T1..15, TResult>(
        string name,
        TContext seed,
        Func<TContext, T1..15, TContext> function,
        Func<TContext, TResult> resultSelector);
}

If SQLite doesn't have a default (or it can be changed with a compile flag), we should probably make the properties nullable. null would mean "don't send a pragma".

We should also look at alternatives to this feature. Using the SqliteConnection.StateChanged event isn't too bad.
Maybe adding properties directly to SqliteConnection would be better along with making calls to AddFunction, etc. re-apply themselves whenever the connection is re-opened.

In my opinion it would be good to have a seperate object which stores the configuration, so that one can initialize different connections with the same configuration.
Regarding mutation after initialization, the question is if this is really necessary. At the moment I see no real use case for this. In order to prevent the mutation we could also package the properties and methods proposed above in some kind of SqliteConnectionConfigurationCreator class and define the SqliteConnectionConfiguration as completely immutable. In order to create the configuration one can add a SqliteConnectionConfigurationCreator.CreateConfiguration() method and/or an constructor  SqliteConnectionConfiguration(SqliteConnectionConfigurationCreator creator).

Why not support all CS keyword from System.Data.SQLite?
Support Foreign Keys=True (especially if it's not default) is a must.
Adding corresponding properties (get/set) to connection object is needed as well. Boolean SqliteConnection.ForeignKeys {get;set;}
Could you tell me how can I enable Foreign Keys currently on 2.0?

@evil-shrike: You can enable foreign keys using the sqlite PRAGMA commands (https://www.sqlite.org/pragma.html):
using (var command = connection.CreateCommand())
{
    command.CommandText = "PRAGMA foreign_keys = true;";
    command.ExecuteNonQuery();
}

My latest thinking on this is that we should probably add a connection pool in version 3.0, and when we do that, we can make the calls to CreateFunction(), etc. persistent.

I am missing the ability to add the items in the connection string that allow you to fully initialize access to a particular database. I do not know of any other driver where you cannot access the database after configuring a proper connection string.
In Microsoft.Data.Sqlite one has to execute a number of PRAGMAs before the connection is actually valid. This breaks many tools such as RoundhousE (http://projectroundhouse.org) which takes a connection string and a set of sql scripts and migrates the database. There is no provision in these tools to allow commands to be run every time a connection is made. For example, in System.Data.Sqlite one can set the Password, Pooling, and many other things that fully set up the connection to the database so that it is ready for action.
It seems that the idea of a connection string is to fully initialize a connection so that work can be performed on the database. Having to execute PRAGMA statements complicates initialization and prevents existing tools from working.
Also I believe from a pooling implementation perspective in other databases, that connections are pooled based on matching connection strings. If the connect string doesn't fully describe the settings of the connection will it not be difficult to determine if a connection can be reused in a particular context?

@jonreis Can you provide specific examples of what breaks in the current implementation?

See #401 for why we didn't add Password.


Alias CREATE TABLE ... to CREATE TEMPORARY TABLE ... during testing

Summary
This would allow us to plop in SLTs from other repos prior to us actually supporting persistent storage.
Blocked on https://github.com/GlareDB/glaredb/issues/844
Specifications
Goals

Not needed anymore


DS1817+ DSM 7.2.1-69057

Hello there,
Have tried to install your latest verion on DS1817+ DSM 7.2.1-69057. Unfortunately with no success. This package is not supported or is not compatible with the latest DSM version. May we please ask you to update the package.
Best regards
TheBjerke

supported, please follow the tutorial，don't forget to execute sudo vgouDaemon fix and reboot twice.

Do still get the same error. Have tested with previous versions of your code as well.

Which tutorial do you refer to?


Cumulative patch

Includes:

C++ exception crashhook Inspired by fishhook project (fix for empty crash log in dynamic library)
Swift exception additional information fetch
Numerous other improvements

@kstenerud please review this PR

Awesome! Thanks for all your hard work :)


Handle "shutdown" requests and "exit" notifications

The "shutdown" request prevously required paramters, but my LSP client (vim-lsc)
doesn't send them, resulting in the error:
StdErr from 'sqls -trace': jsonrpc2: --> request #2: shutdown: null
StdErr from 'sqls -trace': 2020/12/08 15:41:51 error serving, jsonrpc2: code -32602 message:
StdErr from 'sqls -trace': jsonrpc2: <-- error #2: shutdown: {"code":-32602,"message":"","data":null}

Checking the specification1, the params are defined as "void", so I think
checking this is an error?

My LSP client also sends an exit notification2, which wasn't handled:
StdErr from 'sqls -trace': jsonrpc2: --> notif: exit: null
StdErr from 'sqls -trace': 2020/12/08 15:47:52 error serving, jsonrpc2: code -32601 message: method not supported: exit
StdErr from 'sqls -trace': jsonrpc2 handler: notification "exit" handling error: jsonrpc2: code -32601 message: method not supported: exit

So handle this as well.
After these changes sqls shuts down cleanly, without errors and delaying my :q
in vim for ~4 seconds.

@arp242 Thank's for contribution!!! Merged.


#63 fix ruff isort ordering

Adding isort-style sorting to ruff via pyproject.toml
Closes #63
TODOs:

[ ] fix an issue where local code is not sorted correctly and mixed in together with 3rd party imports (ask eddie) @timurcarstensen

Closing since this is already included in a merged PR


More features/scenarios is needed

New scenarios should be identified.
The ones we have currently ;

Security Groups ( this needs to be improved a bit )
Encryption
Tagging


Now can not be used, the server has banned the old version, and now the latest version is 1.3.1

Now can not be used, the server has banned the old version, and now the latest version is 1.3.1

trust me, the author will update it!

Can only believe the author, 25 on the end of the run.

The new version iydsj added IMEI, MAC Adress, Channel, Device Model & Signature to network packets.
You could fix it and send a pull request to us.

it's login post address has been changed into api/v5/login


Add framework-name to agent-name.

When debugging jenkins agent launch failures in multi-jenkins environments, it helps in debugging to have the jenkins-agent task name prefixed with the name of the framework that's launching it.

@kaiwalyajoshi, we have this error now
 org.jenkinsci.plugins.mesos.integration.MesosCloudProvisionTest > runSimpleBuild(JenkinsRule) FAILED
    java.util.concurrent.TimeoutException


Docs.MS Release Updates for azure-template

Update docs metadata and targeting for release of azure-template

Docs Build status updates of commit a97ecc1:
:clock10: Pending: waiting for processors (64 builds ahead of you)
:warning: Docs Build is busy, currently there are 64 builds ahead of this one, for more information you can view the Build queue graph on the Docs Portal.

Docs Build status updates of commit a97ecc1:
:clock10: Pending: waiting for processors (64 builds ahead of you)
:warning: Docs Build is busy, currently there are 64 builds ahead of this one, for more information you can view the Build queue graph on the Docs Portal.

Docs Build status updates of commit a97ecc1:
:warning: Validation status: warnings

File
Status
Preview URL
Details

docs-ref-services/preview/template-readme.md
:warning:Warning
View (azure-node-preview)
Details

ci-configs/packages-preview.json
:white_check_mark:Succeeded

docs-ref-services/preview/template-readme.md

Line 0, Column 0: [Warning-h1-missing] H1 is required. Use a single hash (#) followed by a space to create your top-level heading.
Line 9, Column 16: [Warning-ms-prod-technology-invalid] Invalid value for 'ms.technology': 'azure' is not valid with 'ms.prod' value 'azure'.
Line 2, Column 1: [Suggestion-description-missing] Missing required attribute: 'description'.

For more details, please refer to the build report.
If you see build warnings/errors with permission issues, it might be due to single sign-on (SSO) enabled on Microsoft's GitHub organizations. Please follow instructions here to re-authorize your GitHub account to Docs Build.
Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report.
Note: Your PR may contain errors or warnings unrelated to the files you changed. This happens when external dependencies like GitHub alias, Microsoft alias, cross repo links are updated. Please use these instructions to resolve them.
For any questions, please:Try searching in the Docs contributor and Admin GuideSee the frequently asked questionsPost your question in the Docs support channel

Docs Build status updates of commit a97ecc1:
:warning: Validation status: warnings

File
Status
Preview URL
Details

docs-ref-services/preview/template-readme.md
:warning:Warning
View (azure-node-preview)
Details

ci-configs/packages-preview.json
:white_check_mark:Succeeded

docs-ref-services/preview/template-readme.md

Line 0, Column 0: [Warning-h1-missing] H1 is required. Use a single hash (#) followed by a space to create your top-level heading.
Line 9, Column 16: [Warning-ms-prod-technology-invalid] Invalid value for 'ms.technology': 'azure' is not valid with 'ms.prod' value 'azure'.
Line 2, Column 1: [Suggestion-description-missing] Missing required attribute: 'description'.

For more details, please refer to the build report.
If you see build warnings/errors with permission issues, it might be due to single sign-on (SSO) enabled on Microsoft's GitHub organizations. Please follow instructions here to re-authorize your GitHub account to Docs Build.
Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report.
Note: Your PR may contain errors or warnings unrelated to the files you changed. This happens when external dependencies like GitHub alias, Microsoft alias, cross repo links are updated. Please use these instructions to resolve them.
For any questions, please:Try searching in the Docs contributor and Admin GuideSee the frequently asked questionsPost your question in the Docs support channel


Fixed the issue that missing snx op file blocks the rewards claim

Description
The user can't claim the trading rewards when missing one distribution file.
The PR fixed the above issue. So the user can claim the other rewards if one of the distribution file is missing.
Related issue

Motivation and Context

How Has This Been Tested?
When snx op file is missing, the user can still claim the rewards.
Screenshots (if appropriate):

Approving so we show at least some data, however claim buttons on mobile devices are not working.


[BUG]: Blob key  $audit_general$Audit_General$apac0012 in the url is invalid

Describe the bug
Hi, I am installing the CoE Starter Kit and the [Child] Admin | Sync Logs flow is failing with the error message
"Blob key<PHONE_NUMBER>2216924035945$20201202212432430065720$audit_general$Audit_General$apac0012 in the url is invalid."
The flow stops on the 'Apply to each Cluster' section. Under the GetContentDetials, the INPUTS has the Tenant Id "8a235459-3d2c-415d-8c1e-e2fe133509ad" which does not match our tenant id.

Solution Center of Excellence - Audit Logs
Version <IP_ADDRESS>
App or Flow [Child] Admin | Sync Logs

Expected behavior
I expect the flow to run as the [Child] Admin | Sync Logs flow has all valid connections and the subscription has started.
Screenshots

Please edit the Admin | Sync Audit Logs and configure the Tenant ID with your Tenant ID

Thanks for using CoE

Updating the tenant id resolved the issue.
Thanks


Add new distances

PR Checklist

[ ] Referenced issue is linked
[ ] If you've fixed a bug or added code that should be tested, add tests!
[ ] Documentation in docs is updated

Description of changes

New distance metrics in the perturbation space are implemented

Additional context

In addition, can you add "sum of t-test over all genes" (assuming variables are normally distributed) as a metric, as discussed?
For KL divergence, there should either be a check or (better) a parameter to switch to the version which takes in count data. You should be able to use scipy's entropy for this implementation, I believe.

@mojtababahrami1993 No... I thought there was a fast t-test implementation (where you skip computing the p-value) but we actually don't have it. I would make sure to get the same results as scipy.stats.ttest_ind but simply write it manually so that the additional computation of the p-value is dropped.

For KL divergence, there should either be a check or (better) a parameter to switch to the version which takes in count data. You should be able to use scipy's entropy for this implementation, I believe.

setting a parameter to calculate the KL divergence using count data will not work because you'll easily go to infinity (get a division by zero) when the gene count of the second group is zero for a bin where the first group is greater than zero. You always have to fit a Gaussian/NB distribution first to avoid this.

@mojtababahrami1993 No... I thought there was a fast t-test implementation (where you skip computing the p-value) but we actually don't have it. I would make sure to get the same results as scipy.stats.ttest_ind but simply write it manually so that the additional computation of the p-value is dropped.

Done. Just to mention that I had to sum over the absolute values of the t-statistic across all genes to avoid positive and negative statistics to cancel each out. makes sense?

FAILED tests/tools/_distances/test_distances.py::TestDistances::test_distance[kl_divergence] - assert nan == 0
FAILED tests/tools/_distances/test_distances.py::TestDistances::test_distance[t_test] - assert nan == 0

These tests don't pass yet. Texted @mojtababahrami1993 on Slack

@yugeji @Zethson
The tests are failing due to testing the distances with a subsampled dataset adata_subsampled = sc.pp.subsample(adata, 0.001, copy=True) with 1 sample in each group. This makes the standard deviation of each group zero and some distances equal to nan.
Do you suggest handling such a condition (a group with only 1 sample) in the distance function or just revert testing with the previous real non-sampled data?

@mojtababahrami1993 we have to subsample because else the test takes ages, especially Wasserstein is really slow. I actually thought that I had tested that your implementation breaks even without the subsampling but that's apparently not the case. It would be really good if we could handle such cases with the code because reverting the subsampling is something that I'd really like to avoid.

@mojtababahrami1993 we are now conditionally subsampling:
    @fixture
    def adata(self, request):
        no_subsample_distances = ["kl_divergence", "t_test"]
        distance = request.node.callspec.params["distance"]

        adata = pt.dt.distance_example()
        if distance not in no_subsample_distances:
            adata = sc.pp.subsample(adata, 0.001, copy=True)
        else:
            adata = sc.pp.subsample(adata, 0.1, copy=True)

        return adata

which should ensure that your object has enough samples. A 10% subsample should be doable, right?


Update README URLs based on HTTP redirects

Created with https://github.com/dkhamsing/frankenstein
GitHub Corrected URLs

Was
Now

https://github.com/HarpyWar/pvpgn
https://github.com/pvpgn/pvpgn-server

HTTPS Corrected URLs

Was
Now

http://docs.python.org/2/library/stdtypes.html#str.format
https://docs.python.org/2/library/stdtypes.html#str.format

http://docs.python.org/3/library/string.html#module-string
https://docs.python.org/3/library/string.html#module-string

http://en.wikipedia.org/wiki/Internationalization_and_localization
https://en.wikipedia.org/wiki/Internationalization_and_localization

Other Corrected URLs

Was
Now

http://www.kbengine.org/
http://kbengine.org/

https://code.google.com/p/saddy/
https://github.com/mamontov-cpp/saddy-graphics-engine-2d

Merged, thanks!

:wink: