GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Add extra_returning_fields in apply by kigawas · Pull Request #767 · python-gino/gino

Conversation

@kigawas

@kigawas kigawas commented Mar 17, 2021 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Closes #730

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@kigawas
kigawas force-pushed the extra-returning-fields branch 3 times, most recently from b3be96a to 24aa060 Compare March 17, 2021 08:11
@kigawas
Add extra_returning_fields in apply
c224408
@kigawas
kigawas force-pushed the extra-returning-fields branch from 24aa060 to c224408 Compare March 18, 2021 06:37
@xnuinside

xnuinside commented May 3, 2021 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

@kigawas, hi! Good PR & additional feature.

@wwwjfy, @fantix, what do you think?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@wwwjfy

wwwjfy commented Jun 20, 2021

Copy link
Copy Markdown
Member

Sorry for the late.

I'm thinking of a parameter to return/reload every column instead of specified ones only. It saves a get call, which is probably being used in such case now.

Another nitpicky point is the usage of tuple in this PR. As I understand, list is more suitable here, semantically. Quoting Python doc: Tuples are immutable sequences, typically used to store collections of heterogeneous data

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@kigawas

kigawas commented Jun 20, 2021

Copy link
Copy Markdown
Contributor Author

Yes. Tuple should be replaced by Sequence in Python, which sugguests an immutable sequence although you can still pass list.

I'm thinking of a parameter to return/reload every column instead of specified ones only. It saves a get call, which is probably being used in such case now.

It's certainly better to get all columns by default, but for now a transitional approach would be more realistic to avoid API change

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@wwwjfy

wwwjfy commented Jun 20, 2021

Copy link
Copy Markdown
Member

I don't mean that big a change, but just an additional parameter like update_all (need a good name) to return all columns and update the instance. Similar to the one in the PR, but no need to list every single one.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Extra returning fields in UpdateRequest.apply

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@kigawas @xnuinside @wwwjfy
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.