Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for text type #304

Merged
merged 43 commits into from
Nov 21, 2023
Merged

Add support for text type #304

merged 43 commits into from
Nov 21, 2023

Conversation

sirex
Copy link
Collaborator

@sirex sirex commented Oct 2, 2022

No description provided.

@sirex sirex self-assigned this Oct 2, 2022
@sirex sirex linked an issue Oct 2, 2022 that may be closed by this pull request
13 tasks
It is possible to use text type in manifest files, write and read
to/from database. Added tests for html format, but could not easilly get
it show `name@lang`, now it shows `name.lang`.

Didn't tested it with on changelog, didn't tested how patch and
changelog works.

Didn't tested if it is able to read and push text type from external
sources.
@sirex sirex force-pushed the 204-add-support-for-text-type branch from 957350f to 1fb9dc3 Compare October 2, 2022 18:12
@adp-atea
Copy link
Contributor

  1. It seems that prop@lang is not parsing, when when reading from external source
  2. When selecting app.get(f'/{model}/?select(title@lt)') is not parsing. Also same with prop@lange = value and sort(prop@lang)

Copy link
Collaborator Author

@sirex sirex left a comment

Choose a reason for hiding this comment

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

There are multiple errors see my notes in notes/types/text.sh.

sirex and others added 2 commits February 17, 2023 17:00

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
IngaIVPK added a commit to atviriduomenys/manifest that referenced this pull request Mar 22, 2023
Pasitikslinau su Mantu, tai ne `array` tipo duomenys.
Tai `text` tipo duomenys. Primenu, kad  `text` nepalaikomas, todėl esame sutarę, kad žymime `string` ir nurodome 4 brandos lygį.
atviriduomenys/spinta#304
@adp-atea
Copy link
Contributor

adp-atea commented May 17, 2023

There might be a problem for type push chunks, could not write a decent test and for just added skip wrapper for them. I might be wrong but it seems in our case its not possible to create jsonb format,
sa.dialects.postgresql import JSONB for ex.
``@pytest.fixture(scope='module')
def geo_db_for_text():
with create_sqlite_db({
'salis': [
sa.Column('id', sa.Integer, primary_key=True),
sa.Column('kodas', sa.Text),
sa.Column('pavadinimas', sa.dialects.postgresql.JSONB),
],
'miestas': [
sa.Column('id', sa.Integer, primary_key=True),
sa.Column('pavadinimas', sa.Text),
sa.Column('salis', sa.dialects.postgresql.JSONB),
],
})
It would return render error for test_text_type_push_chunks and test_type_text_push if something like geo_db_for_text would be created.

Also it turn out that some tests were failing because of this commit
1fb9dc3

I looked and master branch and rollback like it was in master (probably override it when merging)
it mostly commit
50b4de8

@sirex
Copy link
Collaborator Author

sirex commented Jul 26, 2023

From given example:

@pytest.fixture(scope='module')
def geo_db_for_text():
    with create_sqlite_db({
        'salis': [
            sa.Column('id', sa.Integer, primary_key=True),
            sa.Column('kodas', sa.Text),
            sa.Column('pavadinimas', sa.dialects.postgresql.JSONB),
        ],
        'miestas': [
            sa.Column('id', sa.Integer, primary_key=True),
            sa.Column('pavadinimas', sa.Text),
            sa.Column('salis', sa.dialects.postgresql.JSONB),
        ],
    })

I see, that you are trying to pass PostgreSQL JSONB column type, to an SQLite database, this will not work, you can't use column types from other dialects, here you must use SQLite types.

I'm not sure, what you are trying to achieve here, but if you try to store text data to database, you should do this on PostgreSQL database, not on SQLite. SQLite is usually used to simulate an external datasource and we do not store data to external data sources, we only store data to internal backends, and internal backend uses PostgreSQL.

@sirex sirex assigned adp-atea and unassigned sirex Nov 21, 2023
@sirex sirex merged commit d4cb389 into master Nov 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for text type
2 participants