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

ITDE: Unify ports for database, BucketFS, and SSH #308

Closed
tomuben opened this issue May 8, 2023 · 10 comments · Fixed by #354
Closed

ITDE: Unify ports for database, BucketFS, and SSH #308

tomuben opened this issue May 8, 2023 · 10 comments · Fixed by #354
Assignees
Labels
refactoring Code improvement without behavior change

Comments

@tomuben
Copy link
Collaborator

tomuben commented May 8, 2023

Background

Currently ITDE uses custom ports for

  • DB (8888)
  • BucketFs (6583)

It would be better from a customer point of view to use the standard ports:

Acceptance Criteria

This ticket also closes #341

@tomuben tomuben added the refactoring Code improvement without behavior change label May 8, 2023
@MarleneKress79789 MarleneKress79789 self-assigned this Jun 7, 2023
@ckunki ckunki self-assigned this Jun 7, 2023
@ckunki
Copy link
Contributor

ckunki commented Jun 7, 2023

Approved from my side, effort negligible

@ckunki
Copy link
Contributor

ckunki commented Jun 9, 2023

Alignment in daily:

  1. remove current branch
  2. re-apply required changes after PR Implemented ticket #329 and fixed authorized_keys #339 has been merged (forward SSH port)

@Nicoretti
Copy link
Member

Nicoretti commented Jun 12, 2023

Approved from my side, effort negligible

Well changing the default's may be negligible, but that's probably not true for all the ITDE call sites (users, projects).
Keep in mind, changing defaults is a breaking change. Don't get me wrong, changing the defaults can be legit but
the ripple effects should be considered.

  • All ITDE dependents potentially also need to do some work

@redcatbear redcatbear added the blocked:yes Currently blocked by another issue label Jun 12, 2023
@redcatbear
Copy link
Contributor

Blocked by: #329

@redcatbear redcatbear removed the blocked:yes Currently blocked by another issue label Jun 21, 2023
@ckunki
Copy link
Contributor

ckunki commented Jun 22, 2023

The files EXAConf in folder docker_db_config_template also contain port specifications:

[Node : 11]
  ExposedPorts = 8888:8899, 6583:6594
[BucketFS : bfsdefault]
    HttpPort = 6583
[DB : DB1]
    Port = 8888

Should we change these as well?

@ckunki ckunki changed the title Use default ports for Db and BucketFS Unify ports for database, BucketFS, and SSH Jun 22, 2023
@ckunki ckunki changed the title Unify ports for database, BucketFS, and SSH ITDE: Unify ports for database, BucketFS, and SSH Jun 22, 2023
@MarleneKress79789
Copy link
Contributor

The files EXAConf in folder docker_db_config_template also contain port specifications:

[Node : 11]
  ExposedPorts = 8888:8899, 6583:6594
[BucketFS : bfsdefault]
    HttpPort = 6583
[DB : DB1]
    Port = 8888

Should we change these as well?

You have to change these in the files that are not sym links(7.0, 7.1, 8.XX) and then run the update_packaging.sh which will update files in exasol_integration_test_docker_environment/docker_db_config. not sure about the > ExposedPorts though

@ckunki
Copy link
Contributor

ckunki commented Jun 23, 2023

Thank to @MarleneKress79789 !
And thanks to @tkilias for discussing and improving which ports to use throughout the code.
Most findings have been addressed.
Please feel free to review!

@redcatbear
Copy link
Contributor

@ckunki neads a pair session with @tkilias to continue.

@ckunki
Copy link
Contributor

ckunki commented Jul 3, 2023

Hi @tkilias on Friday finally I was successful. Could you please review PR #354 again?

@ckunki
Copy link
Contributor

ckunki commented Jul 3, 2023

To ensure compatibility with multiple versions of Exasol database ITDE employs a matrix build executing its tests for each version. Lately the build failed for database version prerelease-8.17.0.

Since version 8.x the size of the Docker Container of Exasol database climbed from 6 GB to more than 10 GB. Additionally we found out that besides the version according to the build matrix the build did also use the latest (default) version of the database. These especially were the integration tests for ITDE's pytest plugin in test/integration/pytest_itde_test.py.

So when ITDE's build matrix specifies version 8.17 and the test additionally downloads the latest version 8.18 this resulted in error message "no space left on device". If the developer's local machine had more disk space the error did not show there.

To fix the problem we added @pytest.mark.skipif to the test cases of ITDE's pytest plugin in order to only run these tests for the latest version of Exasol database — verifying the plugin only for a single version of the database is sufficient.

The easiest way to skip the tests seemed to evaluate the environment variable EXASOL_VERSION. As the tests for ITDE's pytest plugin are written in pytest as well we additionally needed to forward the environment variables to all pytest integration tests in file noxfile.py, method run_tests():

session.run(
        "pytest",
        "--itde-db-version", db_version,
        "./test/integration/pytest_itde_test.py",
        env=env,
    )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Code improvement without behavior change
Projects
None yet
5 participants