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

Update dependencies 2025-01-13 #56

Merged
merged 4 commits into from
Jan 13, 2025
Merged

Update dependencies 2025-01-13 #56

merged 4 commits into from
Jan 13, 2025

Conversation

Nicoretti
Copy link
Member

@Nicoretti Nicoretti commented Jan 13, 2025

No description provided.

@Nicoretti Nicoretti enabled auto-merge (squash) January 13, 2025 13:38
python-version: ${{ matrix.python-version }}

- name: Run Tests and Collect Coverage
run: poetry run nox -s test:integration -- -- --coverage --db-version ${{ matrix.exasol-version }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Will the coverage results be merged with the results for the unit tests?

Copy link
Contributor

Choose a reason for hiding this comment

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

In bucketfs-python we needed to use --itde-db-version.
Which one is correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

Will the coverage results be merged with the results for the unit tests?

yes the results will be merged.

In bucketfs-python we needed to use --itde-db-version.
Which one is correct?

Depends on what tools and versions of those tools are used

Copy link
Contributor

Choose a reason for hiding this comment

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

In bucketfs-python we needed to use --itde-db-version. Which one is correct?

Depends on what tools and versions of those tools are used

Can you explain this a bit more?
Does error-reporting-python use a database at all?
is --db-version correct here?
Why does bucketfs-python need to use --itde-db-version?

Copy link
Member Author

Choose a reason for hiding this comment

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

Why does bucketfs-python need to use --itde-db-version?

Feel free to call me and we have a look at it, but I don't think this is subject of this PR.
That said happy to investigate your issues/question at hand regarding bucketfs-python with yout, but not in the context and PR of another project.

Does error-reporting-python use a database at all?

No, but it does have integration tests

is --db-version correct here?

Should be ok, afik if no DB is requested nothing happens with that parameter.
Keeping it aligned with the template will simplify future udpates though.

python-version: ${{ matrix.python-version }}

- name: Run Tests and Collect Coverage
run: poetry run nox -s test:integration -- -- --coverage --db-version ${{ matrix.exasol-version }}
Copy link
Contributor

Choose a reason for hiding this comment

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

In bucketfs-python we needed to use --itde-db-version.
Which one is correct?

@Nicoretti Nicoretti requested a review from ckunki January 13, 2025 13:51
python-version: ${{ matrix.python-version }}

- name: Run Tests and Collect Coverage
run: poetry run nox -s test:integration -- -- --coverage --db-version ${{ matrix.exasol-version }}
Copy link
Contributor

Choose a reason for hiding this comment

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

In bucketfs-python we needed to use --itde-db-version. Which one is correct?

Depends on what tools and versions of those tools are used

Can you explain this a bit more?
Does error-reporting-python use a database at all?
is --db-version correct here?
Why does bucketfs-python need to use --itde-db-version?

.github/workflows/checks.yml Show resolved Hide resolved

- id: set-matrix
run: |
echo "matrix=$(poetry run nox -s matrix:all)" >> $GITHUB_OUTPUT
Copy link
Contributor

Choose a reason for hiding this comment

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

If I am correct, then nox task matrix:all is defined in PTB.
But still uses database version 7.1.9, which is quite outdated.
As 7.1.30 and 8.32.0 are already available.
Do you plan to update this?
How can a single project override the default settings?
I propose to support using an environment variable here and to simply set it in the workflow file.

Copy link
Member Author

@Nicoretti Nicoretti Jan 13, 2025

Choose a reason for hiding this comment

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

There are multiple ways to do that, for details see the changlog where this was introduced: https://exasol.github.io/python-toolbox/main/changes/changes_0.20.0.html#added

@Nicoretti Nicoretti requested a review from ckunki January 13, 2025 15:03
@Nicoretti Nicoretti merged commit babe4a2 into main Jan 13, 2025
27 checks passed
@Nicoretti Nicoretti deleted the dependencies/2025-01-13 branch January 13, 2025 16:09
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.

2 participants