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.

Sign up for GitHub

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

Jump to bottom

graphql,metadata,db: support queries for tables in all schemas #30

Merged
merged 24 commits into from
Sep 26, 2021
Merged

graphql,metadata,db: support queries for tables in all schemas #30

merged 24 commits into from
Sep 26, 2021

Conversation

kolharsam
Copy link
Owner

@kolharsam kolharsam commented Sep 16, 2021 •
edited

The PR hopes to

resolve #3, resolve #9, resolve #10 and resolve #11

@kolharsam kolharsam self-assigned this Sep 16, 2021
kolharsam and others added 13 commits September 18, 2021 22:44
@kolharsam
tests: add caching support for assets for tests (#31)
b52efeb 
* add caching support for assets for test

* update rust.yml by fixing the syntax err

* set cache_on_failure to `true`

* add clippy workflow

* revert changes

* add cache support for clippy workflow

* change

* update lint action with format

* change name

* update lint check

* update `on` settings for clippy.yml

* add Build and test badges to README

* update README
@kolharsam
some temp metadata changes
09cc35a
@kolharsam
temp changes 2
74cea38
@kolharsam
temp 3
ab36dce
@github-actions
Format Rust code using rustfmt
8d84d48
@kolharsam
Merge remote-tracking branch 'origin/main' into sam/server/metadata--…
3843c50 
…all-schema-query-support
@kolharsam
add new appState for app_data
1f27e26
@kolharsam
add new metadata requests
41c82af
@kolharsam
update schema.sql with new migrate
d3a471d
@kolharsam
update metadata
eb1b430
@kolharsam
update server
6ec99af
@kolharsam
update db
ac54abf
@kolharsam
update main and also tests
49cef44
@kolharsam kolharsam changed the title graphql,metadata,db: support queries from all schemas graphql,metadata,db: support queries for tables in all schemas Sep 22, 2021
@kolharsam kolharsam marked this pull request as ready for review September 22, 2021 16:29
kolharsam
kolharsam commented Sep 22, 2021
View reviewed changes
src/server.rs Outdated Show resolved Hide resolved
@iykekings
Copy link
Collaborator

Code review almost done

iykekings
iykekings reviewed Sep 23, 2021
View reviewed changes
src/server.rs Outdated Show resolved Hide resolved
iykekings
iykekings reviewed Sep 23, 2021
View reviewed changes
src/server.rs Outdated Show resolved Hide resolved
iykekings
iykekings reviewed Sep 23, 2021
View reviewed changes
src/server.rs Outdated Show resolved Hide resolved
iykekings
iykekings reviewed Sep 23, 2021
View reviewed changes
src/server.rs Outdated Show resolved Hide resolved
iykekings
iykekings reviewed Sep 23, 2021
View reviewed changes
src/server.rs Outdated Show resolved Hide resolved
iykekings
iykekings reviewed Sep 23, 2021
View reviewed changes
src/server.rs Outdated Show resolved Hide resolved
iykekings
iykekings requested changes Sep 23, 2021
View reviewed changes
Copy link
Collaborator

@iykekings iykekings left a comment

Choose a reason for hiding this comment

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

@kolharsam I have some refactoring suggestion. It is totally optional to adopt it. I will test the functionality in a bit

@iykekings
Copy link
Collaborator

iykekings commented Sep 23, 2021 •
edited

The Responder we impl for MetadataResponse defaults to StatusCode::BadRequest, but we can return (MetadataResponse, StatusCode) where we need to add a custom status code, since Responder is implemented for (T: Responder, StatusCode). And other goodies that comes with the trait.

@kolharsam
Copy link
Owner Author

kolharsam commented Sep 24, 2021 •
edited

@iykekings These are excellent suggestions! 🙌
Thank you very much! For telling me about this! I shall get back to you once I'm ready with the suggested changes

@kolharsam
Copy link
Owner Author

@iykekings I'll be creating an issue out of this recent suggestion(s) that you've made. To implement the Responder trait for all the other types that we're making responses from throughout the codebase!

@kolharsam kolharsam mentioned this pull request Sep 24, 2021
Closed
@kolharsam
Copy link
Owner Author

@iykekings, could you please approve this PR if there are no other objections from your end?

iykekings
iykekings approved these changes Sep 26, 2021
View reviewed changes
Copy link
Collaborator

@iykekings iykekings left a comment

Choose a reason for hiding this comment

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

LGTM! 😎💯

@kolharsam kolharsam merged commit 38df766 into main Sep 26, 2021
@kolharsam kolharsam deleted the sam/server/metadata--all-schema-query-support branch September 26, 2021 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iykekings iykekings iykekings approved these changes

Assignees

@kolharsam kolharsam

Labels
GraphQL metadata postgres tests 🧪
Projects
None yet
Milestone
No milestone
2 participants
@kolharsam @iykekings