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

Undo arches pin, remove REST framework cruft #69

Merged
merged 5 commits into from
Mar 5, 2025

Conversation

jacobtylerwalls
Copy link
Member

We need to loosen the arches pin to be able to unblock lingo PRs, see failing action

@@ -28,8 +27,6 @@ class Reference:


class ReferenceDataType(BaseDataType):
rest_framework_model_field = JSONField(null=True)
Copy link
Member Author

@jacobtylerwalls jacobtylerwalls Mar 4, 2025

Choose a reason for hiding this comment

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

I'm moving away from a pattern that expects this to exist on the python datatypes in favor of a fully isolated mapping so that API clients don't leak into the python datatypes.

To test this, you'd need to be on archesproject/arches-lingo#240, or we can just let the dust settle tomorrow.

@jacobtylerwalls
Copy link
Member Author

@johnatawnclementawn running tests locally gives me this failure loading polyhierarchical_collections.json. It fails on the constraint I added in archesproject/arches#11792. Was this duplicate always there, or is my check misfiring?

django.db.utils.IntegrityError: Problem installing fixture '/Users/jwalls/prj/arches-controlled-lists/tests/fixtures/data/polyhierarchical_collections.json': Could not load models.Relation(pk=62c053fe-706f-11ef-b739-5f5ed88be48e): duplicate key value violates unique constraint "unique_relation_bidirectional"
DETAIL:  Key ((
CASE
    WHEN conceptidfrom < conceptidto THEN COALESCE(conceptidfrom::text, ''::text) || COALESCE(COALESCE(','::text, ''::text) || COALESCE(conceptidto::text, ''::text), ''::text)
    ELSE COALESCE(conceptidto::text, ''::text) || COALESCE(COALESCE(','::text, ''::text) || COALESCE(conceptidfrom::text, ''::text), ''::text)
END), relationtype)=(00000000-0000-0000-0000-000000000001,00000000-0000-0000-0000-000000000004, hasTopConcept) already exists.

@johnatawnclementawn
Copy link
Member

johnatawnclementawn commented Mar 5, 2025

@johnatawnclementawn running tests locally gives me this failure loading polyhierarchical_collections.json. It fails on the constraint I added in archesproject/arches#11792. Was this duplicate always there, or is my check misfiring?

I am also seeing this. It looks like this is an issue with using fixtures - the initial Arches migrations create the default Arches concepts, but the fixtures also had those concepts/values/relations. I've removed the default Arches concepts from the fixtures

Copy link
Member

@johnatawnclementawn johnatawnclementawn left a comment

Choose a reason for hiding this comment

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

CI will fail until we get a new alpha version for core arches v8. Worth exploring testing matrix to ensure project is tested against v7.6 and v8

@jacobtylerwalls
Copy link
Member Author

I've removed the default Arches concepts from the fixtures

Thanks for fixing it!

@johnatawnclementawn johnatawnclementawn self-requested a review March 5, 2025 18:50
Copy link
Member

@johnatawnclementawn johnatawnclementawn left a comment

Choose a reason for hiding this comment

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

lgtm 👍🏼

@jacobtylerwalls jacobtylerwalls merged commit afefbf8 into main Mar 5, 2025
3 of 5 checks passed
@jacobtylerwalls jacobtylerwalls deleted the jtw/remove-rest-cruft branch March 5, 2025 18:52
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