CICD Integration tests: s3 dataset shares, persistent shares #1580
Conversation
…+ aws clients for dataset
…egration-tests-datasets-pt2 # Conflicts: # tests_new/integration_tests/core/environment/global_conftest.py # tests_new/integration_tests/modules/s3_datasets/global_conftest.py # tests_new/integration_tests/modules/s3_datasets/test_s3_dataset.py
SofiaSazonova
changed the title
| @@ -220,3 +221,15 @@ | |||
| resolver=get_consumption_role_policies, | |||
| test_scope='Environment', | |||
| ) | |||
|
|
|||
|
|
|||
| getConsumptionRole = gql.QueryField( | |||
There was a problem hiding this comment.
nit: to avoid all those changes in the backend you could have used listEnvironmentConsumptionRoles with a filter
| def delete_role(self, role_name): | ||
| try: | ||
| self._client.delete_role(RoleName=role_name) | ||
| except Exception as e: |
There was a problem hiding this comment.
except just for logging isn't really necessary, let the exception bubble up and the runtime will eventually log it with a stacktrace etc
|
|
||
|
|
||
| # it's here and not in Env test module, because it's used only here and we don't want circular dependencies | ||
| def get_environment_access_token(client, env_uri, group_uri): |
There was a problem hiding this comment.
I wouldn't mix that. To avoid the circular dependency I'd add access_token as a parameter in the get_group_session
| updated_before = datetime.strptime(stack.updated, '%Y-%m-%d %H:%M:%S.%f').timestamp() | ||
| stack = update_env_stack(client1, persistent_env1) | ||
| assert_that(stack).contains_entry(status='UPDATE_COMPLETE') | ||
| updated = datetime.strptime(stack.updated, '%Y-%m-%d %H:%M:%S.%f').timestamp() | ||
| assert_that(updated).is_greater_than_or_equal_to(updated_before) |
There was a problem hiding this comment.
- we write ISO dates so you can use
datetime.fromisoformat(...) - you don't have to convert to timestamps, datetime objects are comparable and errors will much easier to read
- depending on whether
updatedis updated based on CFN or not (see my comment below) we might not want the "equal" part
i.e...
nn1 = datetime.datetime.fromisoformat('2024-10-01 08:12:27.732312') # this is the format we write in the db
nn2 = datetime.datetime.now()
assert_that(nn2).is_greater_than(nn1)There was a problem hiding this comment.
- ok
- ok
- We need equal)
There was a problem hiding this comment.
wrt 3 and for the record (not to be solved in this PR).
Ideally we would want to be able to tell wether an update was issued but at the moment I don't think it's possible as this is a limitation of the backend.
| @@ -167,6 +167,11 @@ def session_id() -> str: | |||
| return datetime.datetime.utcnow().isoformat() | |||
|
|
|||
|
|
|||
| @pytest.fixture(scope='session', autouse=True) | |||
| def session_start_timestamp(session_id) -> float: | |||
| persistent_env1.environmentUri, | ||
| target_type='environment', | ||
| ) | ||
| updated_before = datetime.strptime(stack.updated, '%Y-%m-%d %H:%M:%S.%f') |
There was a problem hiding this comment.
nit: you could use datetime.fromisoformat
Feature or Bugfix
Detail
Relates
Security
Please answer the questions below briefly where applicable, or write
N/A. Based onOWASP 10.
fetching data from storage outside the application (e.g. a database, an S3 bucket)?
evalor similar functions are used?By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.