Skip to content

Migrate Directory to within Crosslink repo#669

Open
kurtnordstrom wants to merge 87 commits into
mainfrom
directory-migrate-candidate-2
Open

Migrate Directory to within Crosslink repo#669
kurtnordstrom wants to merge 87 commits into
mainfrom
directory-migrate-candidate-2

Conversation

@kurtnordstrom

@kurtnordstrom kurtnordstrom commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

No description provided.

skomorokh and others added 30 commits November 19, 2024 14:55
@skomorokh
Initial commit
678b1ae
@skomorokh
Actually wire db into api
07db93e
@skomorokh
Bit more fleshed out
84f6dde
@skomorokh
Do left join onto a view so SQLC will embed symbol as nullable
fa1748d
@skomorokh
Use Nullable, strict mode, validation, go generate
8f00a6a
@skomorokh
Schema links for better fuzzing, structured log on context, start PATCH
3f424e7
@skomorokh
...er, meant to amend previous commit
14591fa
@skomorokh
Make symbol spec consistent, name isn't nullable
6be1bd9
@skomorokh
Use tools.go
1cf5c94
@skomorokh
Authority POST and list
a58a920
@skomorokh
App package and migrations
ca9a09f
@skomorokh
Makefile and initial test
6d07c3f
@skomorokh
Flesh out PATCH logic, add naive integration test
56736a7 
Notably adds updating of symbols and moves more of Entry update to happen in Go and simplifies the query
@skomorokh
Use pointers and give up on avoiding separate PATCH schema
d939d6c
@skomorokh
Fix nits and shore up error handling enough for fuzzer to run again
3a549b8
@skomorokh
Use readOnly to simplify api spec
75cb223
@skomorokh
Split impl.go by entity
e548b5b
@skomorokh
Roadmap for eventually intended schema/endpoints
450911b
@skomorokh
Allow lookup by symbol, schema tidying
930772b
@skomorokh
Join tiers and networks onto entries in apiroadmap.yml
e2c9d5c
@skomorokh
Add service endpoints
9b3a404 
Notably includes a helper function "elementHasProperty" to aid in
unrolling joined rows by checking if we've already added the data for
one of the embedded entities.
@skomorokh
Simplify elementHasProperty: field.IsValid() checks for a nil pointer
178882a
@skomorokh
Flesh out integration test
8de8fab
@skomorokh
Update
64d12e9
@skomorokh
DELETE entries
97dfb79
@skomorokh
Make maybeUpdateCol generic and use uuid pointer to avoid pgtype
b470fb3
@skomorokh
consortia endpoint
6e11ef0
@skomorokh
Remove authorities table, we'll manage these like other vocabularies
6cd47c5
@skomorokh
Use same handlers for by-symbol / by-id
e9403a5
@skomorokh
Configs and doc for debugging w/vscode (#3)
22b9814
@kurtnordstrom
kurtnordstrom requested a review from Copilot July 20, 2026 18:35
Copilot AI reviewed Jul 20, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 213 out of 221 changed files in this pull request and generated 4 comments.

Comment thread directory/auth/auth.go Outdated
Comment thread directory/test/env_entry_test.go Outdated
Comment thread directory/Makefile Outdated
Comment thread directory/app/app.go
@kurtnordstrom
kurtnordstrom marked this pull request as ready for review July 21, 2026 12:42
jakub-id
jakub-id reviewed Jul 21, 2026
View reviewed changes
Comment thread broker/adapter/api_directory.go Outdated
Comment thread directory/api.yaml Outdated
Comment thread go.work.sum Outdated
adamdickmeiss
adamdickmeiss reviewed Jul 21, 2026
View reviewed changes

@adamdickmeiss adamdickmeiss left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The top-level README.md should mention the directory directory as it includes, now, an application.

@jakub-id

jakub-id commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

@kurtnordstrom what happened with the existing directory API/schema used by the broker and the mock? Was it moved?

@kurtnordstrom

kurtnordstrom commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

@kurtnordstrom what happened with the existing directory API/schema used by the broker and the mock? Was it moved?

Yes, it is now in the directory-mock folder

@adamdickmeiss
adamdickmeiss self-requested a review July 23, 2026 12:45
adamdickmeiss
adamdickmeiss requested changes Jul 23, 2026
View reviewed changes
Comment thread Makefile Outdated
GO ?= go
SUBDIRS := testutil iso18626 ncip directory sru marcxml httpclient zoom illmock broker
TOOL_SUBDIRS := iso18626 ncip directory sru marcxml broker
SUBDIRS := testutil iso18626 ncip directory directory-mock sru marcxml httpclient zoom illmock broker

@adamdickmeiss adamdickmeiss Jul 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

directory-mock is compiled after directory. If the directory definition changes, then directory will be compiled with old-definitions.

File directory_gen.go is in two places now, one of which is in git. Regardless of whether in git or not, there should only be one generated file.

@adamdickmeiss

adamdickmeiss commented Jul 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Something is up with the directory/Makefile:

[adam@chicken directory]$ make
go tool sqlc generate
go tool oapi-codegen --config=oapi-codegen.yaml api.yaml
go build -v -o directory ./cmd/directory
[adam@chicken directory]$ make
go tool sqlc generate
go tool oapi-codegen --config=oapi-codegen.yaml api.yaml
go build -v -o directory ./cmd/directory

I would expect 2nd make to do nothing.

@kurtnordstrom
kurtnordstrom force-pushed the directory-migrate-candidate-2 branch from 614b8e9 to 9fb5570 Compare July 24, 2026 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jakub-id jakub-id jakub-id left review comments
Copilot code review Copilot Copilot left review comments
@adamdickmeiss adamdickmeiss adamdickmeiss requested changes
@JanisSaldabols JanisSaldabols Awaiting requested review from JanisSaldabols JanisSaldabols is a code owner

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@kurtnordstrom @jakub-id @adamdickmeiss @skomorokh @ihardy