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

ref(eap) Remove old attributes tables #6375

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

evanh
Copy link
Member

@evanh evanh commented Oct 2, 2024

The original meta attribute tables were replaced with slightly different versions.
Remove the old tables.

The original meta attribute tables were replaced with slightly different versions.
Remove the old tables.
@evanh evanh requested review from a team as code owners October 2, 2024 20:21
@colin-sentry
Copy link
Member

isn't this in migration 5 already?

),
]

def backwards_ops(self) -> Sequence[SqlOperation]:
Copy link
Member

Choose a reason for hiding this comment

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

we probably shouldn't have a backwards ops. We don't want to try and recreate this table if the migration fails

Copy link

codecov bot commented Oct 2, 2024

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
1234 1 1233 3
View the top 1 failed tests by shortest run time
tests.migrations.test_runner test_reverse_all
Stack Traces | 6.71s run time
Traceback (most recent call last):
  File ".../tests/migrations/test_runner.py", line 328, in test_reverse_all
    runner.reverse_migration(migration, force=True)
  File ".../snuba/migrations/runner.py", line 382, in reverse_migration
    self._reverse_migration_impl(migration_key)
  File ".../snuba/migrations/runner.py", line 439, in _reverse_migration_impl
    migration.backwards(context, dry_run)
  File ".../snuba/migrations/migration.py", line 181, in backwards
    ops = self.backwards_ops()
          ^^^^^^^^^^^^^^^^^^^^
  File ".../snuba_migrations/events_analytics_platform/0016_remove_old_span_attribute_table.py", line 53, in backwards_ops
    "index_granularity": self.granularity,
                         ^^^^^^^^^^^^^^^^
AttributeError: 'Migration' object has no attribute 'granularity'

To view individual test run time comparison to the main branch, go to the Test Analytics Dashboard

Copy link

github-actions bot commented Oct 2, 2024

This PR has a migration; here is the generated SQL

-- start migrations

-- forward migration events_analytics_platform : 0016_remove_old_span_attribute_table
Local op: DROP TABLE IF EXISTS spans_attributes_meta_local;
Distributed op: DROP TABLE IF EXISTS spans_attributes_meta_dist;
-- end forward migration events_analytics_platform : 0016_remove_old_span_attribute_table




-- backward migration events_analytics_platform : 0016_remove_old_span_attribute_table
-- end backward migration events_analytics_platform : 0016_remove_old_span_attribute_table

When an escape character is used in a string, Python 3.12 will now show a SyntaxWarning.

See here: https://docs.python.org/dev/whatsnew/3.12.html#other-language-changes

This was causing mypy to fail to run. Fix this bug across the codebase.

Also exclude the rust_snuba files (which suffer from this problem) since they are autogenerated.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants