Docs / docs (pull_request) Failing after 1m

please can you tell me more about this test, i’d like to understand why my pull request fails. on my local site, all the tests work.

Hi @HamaBarhamou :wave:

So this is for your PR here: Added #34277 -- Add Conditional WHERE Clause to bulk_create for SQLite and PostgreSQL by HamaBarhamou · Pull Request #17515 · django/django · GitHub

If you click through to the details on the docs build you can see a spelling error:

WARNING: ref/models/querysets.txt:2487: : Spell check: whereasin: whereasin PostgreSQL, you can utilize PostgreSQL-specific syntax and functions..
WARNING: Found 1 misspelled words

So the question is whereasin a word? My quick search in the dictionary suggests whereas is a word, but ``whereasinis not, so rephrasing towhereas in` should solve your issue.

Kind Regards,

Carlton

Edit:

You can run make spelling in the docs folder if you have the dependencies installed.

1 Like

@carltongibson, I believe I have completed the modifications for my recent pull request. I eagerly await your feedback and suggestions for any necessary improvements. Thank you for your support and collaboration

Glad you got the build fixed there! :1st_place_medal: — It looks like you’ve already got several people involved on the review, so I’ll leave it to them at this point. :gift:

1 Like

Tank you. i hope not to wait forever :smile:

I see the Needs documentation flag is set on the ticket

https://code.djangoproject.com/ticket/34277

If you think you’ve addressed all the comments then you can uncheck that and it will appear on the Patches needing review list.

ok thanks for the info. i’ll uncheck it. i’ve already updated the documentation.

Hi @carltongibson . I still haven’t heard back, is that normal?

@HamaBarhamou is it normal that no-one re-reviewed your PR over the weekend? Yes, quite normal.

You have to understand that Django largely runs on volunteer effort (like your own). There are two paid Fellows, but they have a lot to do, and limited capacity.

Currently there are 18 tickets on the review queue of which yours is only one. Someone will get to it, but you’ll need to be patient.

The best thing to do is to make sure you’ve really addressed all the issues, so that it’s in the best state it can be when someone does get to review it.

If then you’re at a loose end, you can use the Patch review checklist to look at one of the other tickets if you like!

Kind Regards,

Carlton

super. Thank you, your answer is reassuring. I’m working on #5929 in the meantime.

1 Like

Hi @HamaBarhamou. I’ve been reviewing your pull request for conditional on conflict update. I’m a volunteer and not paid so reviews and updates come as time allows.

I reviewed again yesterday - there is quite a bit outstanding to do before I think this is ready for the attention of the fellows (unless they’re interested of course!)

Thank you for your efforts on this - don’t be disheartened by the time between updates. Some things take time to ensure that we get the right approach as, once released, issues can be harder to fix.

1 Like

Hi @ngnpope, thank you for reviewing my pull request for conditional update on conflict. I greatly appreciate the time and effort you put into this, especially knowing that it’s on a volunteer and unpaid basis.

I understand that this process can take time, and I am completely prepared to work patiently on the necessary improvements. Your feedback is crucial to ensure that we are taking the right approach. I agree that issues found after publication can be more challenging to resolve, so let’s take the necessary time to get things right from the start.

To move forward, I have posted some clarification questions on GitHub and would be very grateful for your guidance or responses when you find the time. Your advice is invaluable in helping me understand and improve my contribution.

Thank you again for your support and guidance

hi @ngnpope , i did an update. i’m ready for another revision. thanks

merry christmas to you. @felixxm please I’m ready for another revision or maybe we should wait for the new year ?

https://github.com/django/django/pull/17515
Hi @sarahboyce , sorry there are enough posts and I’m now having trouble loading all the posts on github. however I have replied to all your previous comments. but I don’t know if you’ve seen them because of the “pending” mention that appears.
msg git

as far as versioning documentation is concerned, I was guided by @ngnpope 's comments.
For the documentation, it’s @felixxm '.
I don’t want to go round in circles on this question, because I have the impression that you’re not saying the same thing.
all that’s left is the question of documentation. i’d be delighted if @sarahboyce @felixxm and @ngnpope said the same thing.
As for the question of the Ecluded subclass of F, I solved the problem in my last commit. You may have missed it. Here’s the code:

class Excluded(F):
    def resolve_expression(self, *args, **kwargs):
        super().resolve_expression(*args, **kwargs)
        self.output_field = None
        return self

    def as_sql(self, compiler, connection):
        if not connection.features.supports_excluded_expression:
            raise NotSupportedError(
                f"{connection.vendor} doesn't support partial updating rows on "
                "conflicts during INSERT"
            )
        quoted_field_name = compiler.quote_name_unless_alias(self.name)
        sql = f"EXCLUDED.{quoted_field_name}"
        return sql, []

    def __repr__(self):
        return f"{self.__class__.__name__}({self.name})"

    @cached_property
    def output_field(self):
        """Return the output type of this expressions."""
        output_field = self._resolve_output_field()
        if output_field is None:
            self._output_field_resolved_to_none = True
            raise FieldError("Cannot resolve expression type, unknown output_field")
        return output_field

    @cached_property
    def _output_field_or_none(self):
        """
        Return the output field of this expression, or None if
        _resolve_output_field() didn't return an output type.
        """
        try:
            return self.output_field
        except FieldError:
            if not self._output_field_resolved_to_none:
                raise
1 Like

Ah so you have! I’m sorry Hama I missed that! Also I just received the comments now, having another look :eyes:

thank you for your time

Ok, I only have open comments about the docs changes :+1:

You can keep the database flags to be in the backwards incompatible changes for now (I see that was a suggestion from Nick) but I think the model changes and Excluded should be in the Models section of the release notes similar to how the new create_defaults argument was documented here: Django 5.0 release notes | Django documentation | Django

I would still make the doc updates I was suggesting :thinking: I can’t tell if Mariusz wrote this for you (which I think you’re implying) but even if that’s the case, other people can still give feedback.

But I understand if you want to wait for input from Nick and Mariusz :+1:

yes indeed, thank you @sarahboyce for your feedback. @felixxm and @ngnpope I’d like your opinion on this before I start touching the documentation.

Also here is @felixxm 's comment here:

You should use versionchanged , not versionadded . Also version* annotations are for small notes, not for the entire documentation of new features. You should change a signature in docs and document the new argument directly in the bulk_create() docs.

unless I’ve misunderstood.

I don’t see anything in my recommendations that is against Sarah’s suggestions. Besides, I only left initial comments, I’d probably left many new comments in the next run.

In general, please stop pinging reviewers directly and leaving them direct messages. There is no need to ask for another review on all possible channels. You cannot expect immediate replies. You should read the following FAQ before you will consider doing this again:

Last but not least, don’t leave comments in French.