Not Found for url: https://huggingface.co/google/realm-cc-news-pretrained-openqa/resolve/main/block_records.npy

Traceback (most recent call last):
File "run_finetune.py", line 305, in
main(args)
File "run_finetune.py", line 127, in main
openqa = get_openqa(args, config)
File "/home/chuanqi.tcq/qwk/pytorch-realm-orqa/model.py", line 99, in get_openqa
retriever = RealmRetriever.from_pretrained(args.checkpoint_pretrained_name)
File "/home/chuanqi.tcq/anaconda3/envs/MRC-KI/lib/python3.7/site-packages/transformers/models/realm/retrieval_realm.py", line 114, in from_pretrained
repo_id=pretrained_model_name_or_path, filename=_REALM_BLOCK_RECORDS_FILENAME, **kwargs
File "/home/chuanqi.tcq/anaconda3/envs/MRC-KI/lib/python3.7/site-packages/huggingface_hub/file_download.py", line 545, in hf_hub_download
local_files_only=local_files_only,
File "/home/chuanqi.tcq/anaconda3/envs/MRC-KI/lib/python3.7/site-packages/huggingface_hub/file_download.py", line 352, in cached_download
r.raise_for_status()
File "/home/chuanqi.tcq/anaconda3/envs/MRC-KI/lib/python3.7/site-packages/requests/models.py", line 960, in raise_for_status
raise HTTPError(http_error_msg, response=self)
requests.exceptions.HTTPError: 404 Client Error: Not Found for url: https://huggingface.co/google/realm-cc-news-pretrained-openqa/resolve/main/block_records.npy

Contacting with HF now.


[Storage] Fix live tests azdatalake

[ ] The purpose of this PR is explained in this or a referenced issue.
[ ] The PR does not update generated files.

These files are managed by the codegen framework at Azure/autorest.go.

[ ] Tests are included and/or updated for code changes.
[ ] Updates to module CHANGELOG.md are included.
[ ] MIT license headers are included in each file.

/azp run go - azdatalake

/azp run go - azdatalake

/azp run go - azdatalake


Question: Guide for use in bundled IntelliJ plugin

What version of Delve are you using (dlv version)?
0.11.0-alpha
What version of Go are you using? (go version)?
go1.7.4
What operating system and processor architecture are you using?
darwin-amd-64x
What did you do?
N/A
What did you expect to see?
N/A
What did you see instead?
N/A

I've read in many official places that Delve is packaged with IntelliJ's Go package, and therefore a part of the Gogland EAP - The Golang IDE from Jetbrains. Note that I have installed delve independently of the IDE.
I can't find any guides anywhere on how to actually use or config delve from the IDE.
Could anybody supply any videos/reading material? I can't even find any documentation in this repository...

Never mind, I figured it out.
For those interested, you must simply click the debug button.
To verify, search the output for "dlv".


litmus-portal subscriber crashloopbackoff due to CLUSTER ID MISMATCH

What happened:
Deployed litmus-portal 1.13.0 using helm chart in a very fresh installed Kubernetes cluster, basic workflow worked well.
But the following error occurred against subscriber when i removed and reinstalled the same portal chart on the same cluster again. it was CrashLoopBackOff and complained "CLUSTER ID MISMATCH", meanwhile the self-cluster is not available on the portal.
kubectl logs pod/subscriber-5f7bb775d7-2skwr -n litmus
2021/03/07 09:09:02 connecting to ws://<IP_ADDRESS>:30478/query
2021/03/07 09:09:03 Cluster Connect Established, Listening....
time="2021-03-07T09:09:03Z" level=fatal msg="gql error : {\"payload\":{\"errors\":[{\"message\":\"ERROR:  CLUSTER ID MISMATCH\",\"path\":[\"clusterConnect\"]}],\"data\":null},\"type\":\"data\"}\n"

What you expected to happen:
all related resources are up and running. self-cluster can be correctly identified and confirmed.
The litmus-portal helm chart can be successfully removed and redeployed.
How to reproduce it (as minimally and precisely as possible):

install litmus-portal using helm chart 1.13.0
execute some workflow on the self-cluster
helm uninstall the chart and clean up all left-over resources (e.g. argo workflow controller )
reinstall the same chart again
login and trigger workflow on the portal and see if the self-cluster is available.

Anything else we need to know?:

When you uninstalled the portal it was not properly cleaned maybe. Can you try redoing it, but make sure to delete the litmus namespace all together before reinstalling. There are some configmaps which if not removed properly can cause this error after a reinstall.

When you uninstalled the portal it was not properly cleaned maybe. Can you try redoing it, but make sure to delete the litmus namespace all together before reinstalling. There are some configmaps which if not removed properly can cause this error after a reinstall.

This is the configmap that might be causing this issue litmus-portal-config

you are right. i forgot to clean the cm. the litmus-portal was successfully reinstalled after everything is cleaned up in litmus namespace.  thanks much.

closed this issue. thanks


Bug of group2ctx? Wrong device placement?

For the following code:
x = mx.symbol.MyOp()
exe = x.bind(mx.gpu(), {}, group2ctx={"a": mx.cpu(), "b": mx.gpu()})
exe.forward()
where MyOp is a custom operator written in c++, which prints "CPU" if it is run in cpu context, or prints "GPU" if run in gpu context. MyOp has no input.
I don't use mx.AttrScope to specify the group of x, so default context should be used for x. However, the above code prints "CPU", which means that x is run in cpu context. Why?
If I set group2ctx={"b": mx.cpu(), "a": mx.gpu()}. Then it prints "GPU".
After more tests, I found that the group that has the alphabetically smaller name is chosen for x. Very strange.
Is this a bug? How device placement works?

During PlaceDevice, if an op can't be assigned a device id, its device id will be set 0. And to assign a default context is done after PlaceDevice, so it will not works actually. 0 indicates which context depends on the group2ctx. More deeply, it is the index when traversing a map, so alphabetical. It is should be a bug. @tqchen @piiswrong

@formath What do you mean by "if an op can't be assigned a device id"? How op's device id is assigned?

Depends on the device id of its first control node or input node. Your op may not assigned by this.

@formath So if a symbol a has input b, and b is assigned to cpu device, then a will also be automatically assigned to cpu device regardless of the default context? Why it is designed in this way? This is surprising and no documentation for this is found.

exe = x.bind(mx.gpu(), {}, group2ctx={"a": mx.cpu(), "b": mx.gpu()}) The default context is just indicate this executor run on this context, but not the node or op in symbol.

@formath Is executor just a scheduler? What does it mean by "executor run on this context"? What executor runs? Parameter updating?

@apache/mxnet-committers: This issue has been inactive for the past 90 days. It has no label and needs triage.
For general "how-to" questions, our user forum (and Chinese version) is a good place to get help.


Update CircleCI golang image to v1.11.1

Fixes #873
Signed-off-by: Steve Sloka<EMAIL_ADDRESS>

Missed the PR already in flight, closing this one.


WIP: Service Provider Support

Working on adding in Service Provider support for more flexibility with third party packages.
I've refactored things a bit to use a global Application class where service providers are registered.  The application class now maintains the server instance, so main.swift in apps would need to be updated to look like this:
let application = Application()
// application.register(SomeServiceProvider.self)
application.boot()
application.server.run(port: 8080)

TODO

[ ] Gather feedback
[ ] Test coverage

Example
Using VaporStencil as an example, instead of having to include View.renderers[".stencil"] = StencilRenderer() in main.swift, we could create StencilServiceProvider:
import Vapor

public class StencilServiceProvider: ServiceProvider {

	public override func register() {
		View.renderers[".stencil"] = StencilRenderer()
	}

}

And in main.swift, it would look like:
let application = Application()
application.register(StencilServiceProvider.self)

See original PR https://github.com/qutheory/vapor/pull/39 for more information.

@tannernelson Here you go, had to re-submit the PR as I'm not aware of anyway to change the source branch in an existing PR.

@tannernelson The changes you made in f07998a and 096bc62 tie providers directly to the start of the server.  I separated the app/provider booting away from the server start so vapor-console could boot the app and use providers without starting the server.  With those changes providers are essentially inaccessible to console.

It also looks like those changes dropped the ability to specify a different server driver, unsure if that was intentional or not.

@shnhrrsn Ah okay, I will put those back.

I updated the middleware and providers to have a similar API. I think using a simple array is best because the user can choose to append to that array, or to replace it with an empty array.
let app = Application()

app.get("stencil") { request in
    return try View(path: "template.stencil", context: [
        "hello": "world"
    ])
}

app.middleware.append(ErrorMiddleware) //look at ErrorMiddleware below
app.providers.append(StencilProvider)

app.start(port: 8080)

Also, with everything converted to Request.Handler we can now create Middleware for handling errors like so.
enum Error: ErrorType {
    case NotFound
}

class ErrorMiddleware: Middleware {
    class func handle(handler: Request.Handler) -> Request.Handler {

        return { request in

            do {
                let response = try handler(request: request)
                return response
            } catch Error.NotFound {
                return try Response(status: .NotFound, json: [
                    "error": true,
                    "message": "Item not found"
                ])
            }

        }
    }
}

So now you can define custom Error types and then throw them wherever you want.

Let me know what you think of those changes / if I destroyed any other functions that you want put back.

Streamlined Middleware + Provider APIs look good!
I think the changes you made should be fine, going to update everything else to match and I'll report back if I run into any issues.

I ran into an issue with the latest changes: in vapor-console, there's the ServeCommand. This is going to cause every provider to get booted twice, first when Console tells them to and then again when serve tells the app to start the server.
We could solve this one of two ways:

Re-add the booted flag and add a guard to bootProviders
Merge vapor-console into the main framework and actually run through serve command, which would clean up the process arg looping in Application.start(), since console commands have built in support for options/arguments.

Is there any issues with booting multiple times?

I think it would depend on how complicated the providers get, for the stencil provider there's just a bit of double allocation going on for StencilRenderer which isn't a huge deal.
If we look forward and consider something like events being added to the framework, every time a provider subscribes to one it'll get subscribed twice and now that I think about it.
Now that I think about it, while events right now are a theoretical feature, if they're built on top of something like NSNotificationCenter, this would be the use case for having provider instances instead of using static calls since a provider would need to be allocated to work with NSNotificationCenter.

I think in that case the provider could just implement an initialized object as a static member variable. Then the NSNotificationCenter could delegate to that object.
I think we will be fine with double booting on the console for now. I'd rather put effort into merging the console into the main framework.

I'm good with that.
Have we finalized this architecture then or is there still more changes you want to make?

I'm happy with it. Let's merge tomorrow morning to give @LoganWright and @mpclarkson a chance to look it over.

Hey all, I just rebased this on top of #47 so we can keep the history cleaner.  There were a bunch of conflicts to resolve, so if going forward, we could use that branch, it'd be easier to not have to repeat ourselves.
I can also push to this branch directly if you'd prefer

Moved to #52.


Remove dependency on Folly in TurboModuleUtils.h

Summary
The TurboModuleUtils.h includes "folly/Optional.h" which is not used and creates an unnecessary dependency on Folly.
In this PR we remove this unnecessary include.
It is required for the https://github.com/microsoft/react-native-windows/pull/6804 where we add an experimental support for the C++ TurboModules. While the C++ TurboModules use the same JSI and TurboModule code defined in react-native, we provide a layer that let them to work over the ABI-safe Microsoft.ReactNative.dll boundary. The RNW Nuget distribution with DLL files includes a few source files to create native/turbo modules that work through the ABI-safe API. The TurboModuleUtils.h is one of such files. By removing the dependency on Folly we reduce requirements for the native module code. After this PR is merged we will remove the fork of the TurboModuleUtils.h  added in https://github.com/microsoft/react-native-windows/pull/6804.
Changelog
[Internal] [Fixed] - Remove dependency on Folly in TurboModuleUtils.h
Test Plan
The change does not bring any functional changes. It may only affect code compilation where some code may depend on TurboModuleUtils.h when it needs the "folly/Optional.h". The fix is add the #include <folly/Optional.h> there explicitly.
I had run the iOS tests and they passed:
yarn
pod install in packages\rn-tester
./scripts/objc-test.sh test

@fkgozali , thank you for the quick merge!

@fkgozali , thank you for the quick merge!


Resource availability

Description of change

Fixes https://github.com/meroxa/turbine-project/issues/85
Blockers

[x] merge of https://github.com/meroxa/turbine-go/pull/53
[x] bump to latest version of turbine-go
[x] bump to latest version of turbine-js: https://github.com/meroxa/turbine-js/pull/94
[x] make build to auto-generate docs

This is listing resources correctly for Turbine apps written in

[x] Python
[x] Go
[x] JS

Type of change

[x]  New feature
[ ]  Bug fix
[ ]  Refactor
[ ]  Documentation

How was this tested?

[x]  Unit Tests
[x]  Tested in staging
[ ]  Tested in minikube
[x]  Tested manually

Demo

@jayjayjpg let me know if you need help releasing this to a newer version of our CLI and if this needs to be coordinated with a changelog (happy also to help on that).


Fast datapath preparations

These are the changes to the existing router code to prepare the way for fast datapath.  I didn't include the short peer ID changes, as they are much more tailored to ODP specifics, and so they make more sense as part of the fast datapath PR.
I think this is clean enough for a review pass.  There is still one point I'm not quite happy with yet:  The names "IntraHost" and "InterHost" were intended as stand-ins, until I came up with better names.  But I still haven't come up with replacements that seem obviously better (the best so far is PacketCapture and PacketTransport but they are longer, and PacketCapture isn't right because that interface includes sending, plus PacketCapture has too much affinity with pcap).  I'm hoping inspiration will strike by the time this is merge-ready, and welcome any suggestions.

(github is having a hard time dealing with the extent of the rebasing that has gone on in this branch.  It's probably easier to look at the branch itself.)

I've become concerned that 9e2a79d816 adds more information to the unencrypted handshake.  In fact, very little of the information in the handshake is required to establish encryption.  The downside of moving that information out of the initial handshake is that, in the encrypted case, we will need a second encrypted handshake (in the unencrypted case they can be combined).  But I'm not sure that avoiding an extra round trip is justified in this context.

I've become concerned that 9e2a79d adds more information to the unencrypted handshake.

Don't be. Let's fix https://github.com/weaveworks/weave/issues/1029.

Any objections to me cherry-picking be13eaf and 80e6cbe onto master?

Any objections to me cherry-picking be13eaf and 80e6cbe onto master?

No.  (I think it's unnecessary, but you knew that already.)

cherry-picking be13eaf and 80e6cbe

Done.

re the PeerSummary change... (putting this here so it doesn't get lost in a rebase)...

can we come up with a better name than PeerSummary? That name made sense when the summary was used only for encoding, but it looks really odd for Peer to contain PeerSummary.
what all the test code really wants is a Peer.NewPeerFrom(p *Peer)
NewPeer should call NewPeerFromSummary

can we come up with a better name than PeerSummary? That name made sense when the summary was used only for encoding, but it looks really odd for Peer to contain PeerSummary.

I don't really see this.   It is to be expected that a PeerSummary can be derived from a Peer; in this case, that happens to be by way of a straightforward projection, so we can express it concisely in go (for once).  That seems quite natural and logical to me, so I might have difficulty coming up with another name.

I still cringe every time I read IntraHost and InterHost. Perhaps @squaremo, our naming czar, can come up with something better?

I still cringe every time I read IntraHost and InterHost. Perhaps @squaremo, our naming czar, can come up with something better?

As I noted, they were and are intended to be stand-ins.  Unless you had said "actually they seem fine to me", my plan is to change them.

when? I am close to being happy with the IntraHost changes, except for the name.

Shrug.  It's just a naming thing, so if it is the last thing I fix, that's fine.  It's not like it's the only thing keeping this from being ready to merge.

I was planning to cherry-pick the IntraHost change.

You'll have to wait until the branch as a whole is ready.

Are you planning to make changes other than in response to my review comments?

Of course, if you have further feedback on the other commits, that would be much appreciated.  Whether intrahost is renamed or cherry picked or not will not alter their substance.

Are you planning to make changes other than in response to my review comments?

The "features" commit will change, or possibly go away entirely, due to the handshake changes I'm working on.
Other than that: No, I don't have any further changes planned.

I may need something like https://github.com/weaveworks/weave/blob/master/docs/architecture.txt to get through this. Ideally with some diagrams.

The spirit of the code hasn't changed, and I have tried to make it more readable than before (this is not to criticise the readability before, but the UDP forwarding code was scattered throughout the codebase, and some of the objects / goroutines / layers have also been consolidated).  So if it didn't require a document before, and does now, I have failed, and I will need to figure out how.  If you can point me to something that seems obscure, just as a starting point, that would be great.
Alternatively, if what you are saying is "that new code might be perfectly readable, but it would always have been nice if we had a document describing how the UDP forwarding works" I'm not sure if I would agree or not.  But maybe the fact that we have got by without such a thing so far suggests that the need cannot be that pressing?  In which case it can be filed as a separate issue.

I am finding the control flow much harder to follow that before. Part of that is obviously due to familiarity with the previous code base.

I am finding the control flow much harder to follow that before. Part of that is obviously due to familiarity with the previous code base.

It seems like you are looking at the diff of the whole branch. It's certainly necessary for the code to be comprehensible on that basis, and I would like to work out what we can do to address that.    In particular, while the commit that introduces FlowOp is a pure refactoring and in a sense quite straightforward, it is undeniable that it convolutes things.  Of course, that is the crucial convolution required to implement fast data path in terms of the existing router code base.

I actually have mostly been doing that. But, as you have pointed out on a number of occasions now when I had questions on specific commits, some of the design decisions in earlier commits are motivated by what comes later.
It would help me a lot if I was able to merge all but the interhost/sleeve and flow changes.

I actually have mostly been doing that. But, as you have pointed out on a number of occasions now when I had questions on specific commits, some of the design decisions in earlier commits are motivated by what comes later.

The commits are related but separate, like chapters in a book.  They are not intended to stand alone, but neither should their separation be discarded.  Sometimes the answer to a question "why is this done like this" will be in terms of what happens in a later commit.  In such cases, you won't be able to fully evaluate that explanation until you reach the later commit.  But the separation is important, particularly when trying to understand the changes in terms of the prior state of the code base.  That is why I divided the commits up, and included extended commit messages on the more interesting ones to provide rationale and context.

It would help me a lot if I was able to merge all but the interhost/sleeve and flow changes.

Ok, let's do this synchronously then.  I'll let you know when I think everything is ready to merge.

Naming idea: IntraHost -> Bridge, InterHost -> Overlay.  Pros: short, accurate in the context of the router codebase.  Cons: pretty ambiguous outside of that context (but I think they fit well enough to override that concern).

Naming idea: IntraHost -> Bridge, InterHost -> Overlay

SGTM

Naming idea: IntraHost -> Bridge, InterHost -> Overlay

+1

what all the test code really wants is a Peer.NewPeerFrom(p *Peer)

Done

NewPeer should call NewPeerFromSummary

Done

can we come up with a better name than PeerSummary? That name made sense when the summary was used only for encoding, but it looks really odd for Peer to contain PeerSummary.

Not done.  I still find PeerSummary reasonable here.  Its use is now broader than it was before, but that doesn't make it a bad name.

Other items of feedback that github has now swallowed have also been addressed.

What's the protocol compatibility situation? If we merged this pre 1.1, would it (or could easily be made to) be compatible with 1.0?

A general note about this PR: The commits are not currently protocol-compatible with 1.0/master.  My expectation is that it will be possible to make them compatible with a couple of hours of work.  But I'm deferring that work in case it turns out that FDP proper fundamentally breaks compatibility somewhere else (I'm not aware that it does, but it was not implemented with compatibility in mind, and there are some areas that will require careful thought before declaring that it is compatible).

What's the protocol compatibility situation? If we merged this pre 1.1, would it (or could easily be made to) be compatible with 1.0?

Oh, I was just writing about that.  Yes, I expect so.  Do you want to merge for 1.1? I don't know of a good reason not to, but I don't think we gain anything by doing so, so my inclination is not to merge.

@dpw Is there any architecture document that I could use as an introduction to this PR?

@dpw Is there any architecture document that I could use as an introduction to this PR?

While some implementation aspects of the router change, the architecture does not change, and as far as I am aware, and existing architecture documents continue to apply.  Feedback about any places where the implementation is unclear or unnecessarily obscure is welcome though!

(This morning it occurred to me that the Peer / PeerSumary change might not be gob-compatible.  The gob documentation is quite unclear on such matters.  So I have checked.  It is compatible.)

I've spent a few hours reading this so far; initial thoughts:

The chosen abstractions (bridge/overlay + router core currying) seem reasonable and uncontroversial. No superior alternatives spring immediately to mind, but I will spend some time thinking about it
New naming + commit structure is good. Extended commit comments are helpful
The commit that introduces overlay & sleeve requires the most attention. I'm going to go back and refamiliarise myself with the existing implementation before tackling that in detail

The commit that introduces overlay & sleeve requires the most attention. I'm going to go back and refamiliarise myself with the existing implementation before tackling that in detail

Great.  I did find a small bug there earlier this week, but I didn't get around to pushing the fix yet.  Shall I hold on to it to give you something to look for?

I did find a small bug ... Shall I hold on to it to give you something to look for?

Is it the only one then? :wink:

It may just be me, but the name ConsumePackets on overlay & bridge made me think that they would be synchronous when in fact they start goroutines that continue invoking the supplied callback after the function has returned.

I have similar (mild) concern about Overlay.MakeForwarder - it causes me to wonder if it's doing something similar to Go's make function, and it doesn't sound asynchronous. StartForwarder? GoForward?

I have similar (mild) concern about Overlay.MakeForwarder - it causes me to wonder if it's doing something similar to Go's make function, and it doesn't sound asynchronous. StartForwarder? GoForward?

I see your point about ConsumePackets, but the comment about MakeForwarder seems like a misapprehension of the intent.  MakeForwarder, uhmm, makes a forwarder.  And essentially, a forwarder is something you can call Forward on to relay a packet to the peer concerned.  A forwarder may, as an implementation detail, have goroutines and other active machinery behind it, but it would be wrong to reflect that in the name.  In contrast to ConsumePackets, the asynchrony is not key to the interface.  The only asynchrony from the caller's point of view is indirectly through the forwarder's SetListener method.

MakeForwarder, uhmm, makes a forwarder. And essentially, a forwarder is something you can call Forward on to relay a packet to the peer concerned. A forwarder may, as an implementation detail, have goroutines and other active machinery behind it, but it would be wrong to reflect that in the name.

If the forwarder were completely inert unless you used it to Forward something (which it may accomplish with a goroutine at its discretion), I would agree. That is not the case here - there is a continuous UDP heartbeat process going on in the background. The presence of a Close() method on OverlayForwarder is also indicative IMO.
The aforementioned notwithstanding, why MakeForwarder and not NewForwarder?

If the forwarder were completely inert unless you used it to Forward something (which it may accomplish with a goroutine at its discretion), I would agree. That is not the case here - there is a continuous UDP heartbeat process going on in the background. The presence of a Close() method on OverlayForwarder is also indicative IMO.

The UDP heartbeat is an implementation detail of the sleeveForwarder.  The point of the Overlay interface et al is to hide such details.  And the fast data path code in the "preview release" demonstrates that OverlayForwarder can be implemented without any associated goroutines, etc.

The aforementioned notwithstanding, why MakeForwarder and not NewForwarder?

There seems to be a golang convention that NewForwarder is a function that allocates a concrete type called a Forwarder.  You might say that convention only applies to package-level functions.  But it seemed better not to allude to it here.  NewForwarder might tend to suggest a passive allocation, and, as you point out, implementations might be doing much more than that.  Hence I chose a name that I regard as more neutral.

The UDP heartbeat is an implementation detail of the sleeveForwarder. The point of the Overlay interface et al is to hide such details. And the fast data path code in the "preview release" demonstrates that OverlayForwarder can be implemented without any associated goroutines, etc.

I am persuaded :+1:

Hence I chose a name that I regard as more neutral.

Except there is the make builtin. How about OpenForwarder to mirror OverlayForwarder.Close()?

Except there is the make builtin

Given the rather specialized purpose of the make keyword, and the capitalization difference, and the fact that here the word Make appears in a longer identifier, I'm not sure what kind of confusion can arise.
There's a new keyword too, and that clearly doesn't preclude the use of the word New in identifiers.
There are quite a few uses of [mM]ake in identifiers in the go source tree.

How about OpenForwarder to mirror OverlayForwarder.Close()?

OpenForwarder might suggest the creation of some kind of connection or channel associated with the forwarder (as when one opens a TCP socket).  That's not the case for any implementation I have in mind.  How about I rename Close to Stop or Terminate?

How about I rename Close to Stop or Terminate?

:+1:

sleeve.go is by far the biggest individual file now as it contains then entire UDP forwarding mechanism - do you have any plans to address this post FDP merge? Is the entire ODP overlay in a single file too? Am wondering about the use of packages or some other organising principle (e.g. bridge_pcap.go/bridge_odp.go, overlay_sleeve.go,overlay_sleeve_forwarder.go/overlay_odp.go)

sleeve.go is by far the biggest individual file now as it contains then entire UDP forwarding mechanism - do you have any plans to address this post FDP merge?

I don't have any such plans, although that is certainly not to say that it should not be addressed.
I don't consider the length of sleeve.go to be much of a problem at this point.  There is a high level of cohesion among its contents, and coupling to the rest of the code base is reasonably loose, so it forms a nice unit.  As file boundaries have no semantic significance in golang, it clearly could be split up into multiple files, but that would tend to diminish its status as a component with a well-drawn boundary.
(So, for example, I find it much easier to navigate the UDP encapsulation code now that it is all in one file, albeit a rather large one, compared to the previous state where it was spread out over multiple files that often had other concerns.)
Of course, at what point a file becomes unmanageably long is a subjective matter.  I'm not sure if one can take the go source tree as an authority, but that contains a number of files of similar or greater length (even excluding test files and syscall definition files and other such special cases).

Is the entire ODP overlay in a single file too?

Yes.  It's not currently as long as sleeve.go, though how long it will end up remains to be seen.

Am wondering about the use of packages or some other organising principle

I think that if the router were to be decomposed into multiple packages, with sleeve.go in its own package, I would feel more comfortable with splitting it up into multiple files (as the conceptual boundary would be represented by the package boundary, there would be no need to represent it by a file boundary).
But as no other part of the router is in its own package, doing so would represent a noteworthy step.  I'm not sure the need here is great enough to justify crossing that line.  I certainly don't think there's a strong case to do it before merging.  After merging, the decision can be made at leisure.

(e.g. bridge_pcap.go/bridge_odp.go, overlay_sleeve.go,overlay_sleeve_forwarder.go/overlay_odp.go)

I'm sceptical of the advantages of such a split.  Maybe I just don't like typing long filenames.

I too appreciate the entirety of the sleeve implementation being in a single place - I find it much easier to understand. The concern is less about the length of the file, and more about being able to see the pieces that comprise sleeve (and its FDP counterpart) without trawling through the file; it would be nice if we could have the best of both worlds. I think we're in agreement that fine grained packages would achieve this, but that it is a fair amount of work; the idea of the overlay_$IMPL_$SUBCOMPONENT.go approach was a lightweight compromise. Related, parts of crypto.go are entirely sleeve specific...

and more about being able to see the pieces that comprise sleeve

I think that is a fine wish when those pieces are intended to be taken in isolation.  But that is not the case here.  The "pieces" of sleeve.go are somewhat tightly coupled.  Moving them into separate files as-is would suggests more of a boundary than is justified.  If they were moved into separate files, it would be appropriate to restructure the code to make them less tightly coupled.  But that creates work, for the author, the reader, and those who maintain the code.  At around a thousand lines, I don't think there is a problem that needs solving here.

Related, parts of crypto.go are entirely sleeve specific...

Other parts of the router codebase interact with crypto.go, and the interface between crypto.go and sleeve.go is narrow and simple.  So the file boundary there corresponds to a clear and clean design boundary.

I've just run the smoke tests again for the first time after the most recent rebase, and there are some things that no longer pass:
>>> Ran 34 tests, 5 failed
>>> Fail ./250_dns_lookup_2_test.sh
>>> Fail ./160_claim_3_test.sh
>>> Fail ./210_dns_cross_hosts_3_test.sh
>>> Fail ./660_proxy_ipam_2_test.sh
>>> Fail ./120_discovery_3_test.sh

The coverage report is interesting - sleeve.go is pretty well covered, except for the manual fragmentation path. Interestingly that is covered on current master due to @rade's new 105_frag_2_test.sh so we should get some coverage there post upcoming rebase...

Rebasing should fix 160.

@dpw just to put our conversation yesterday on the record - from a design/maintainability/handover POV I am ready to merge this PR; remaining activity is concerned with convincing myself that the behaviour hasn't changed, but I think that can be time bounded by however long it takes you to rebase + address outstanding comments.

docs/architecture.txt needs updating - it speaks of two separate forwarder goroutines

We have a mix of ways we're testing or setting the value of heartbeatInterval:
759:	if fwd.heartbeatInterval != 0 {
800:		if fwd.heartbeatInterval != time.Duration(0) {
836:	if fwd.heartbeatInterval != SlowHeartbeat {

767:	fwd.heartbeatInterval = time.Duration(FastHeartbeat)
837:		fwd.heartbeatInterval = SlowHeartbeat

As you suggest, unless the outgoing netdev has an MTU of 65535, we'll get an EMSGSIZE anyway, and then we initiate the EPMTU search.

I've had a look at the kernel source and it uses the device MTU in the absence of a discovered PMTU value so this is guaranteed to happen.

By this point, the kernel's TCP code will have established the PMTU to the remote peer's IP, and that works even in the absence of frag-needed responses

Is that Packetization Layer Path MTU Discovery?

I haven't checked, and it's been a while since I wrote that code, but I'd be surprised if it doesn't just recapitulate what the existing code does.

I have, and it does :smile:

Shouldn't MaxUDPPacketSize in router/consts.go be 65535 and not 65536? (it's wrong on master too...)

Name change suggestion for clarity:
sleeveForwarder.effectivePMTU -> sleeveForwarder.overlayPMTU
msgTooBigError.PMTU -> msgTooBigError.underlayPMTU
Not sure what to do with sleeveForwarder.epmtu* though :frowning:

I made this diagram to understand the PMTU discovery - I think it would make a helpful comment in sleeve.go:
+----------+----+-----+--------+--------+----------+-------------------------+
| Ethernet | IP | UDP | Sleeve | Sleeve | Overlay  | Overlay Layer 3 Payload |
|          |    |     | Packet | Frame  | Ethernet |                         |
|          |    |     | Header | Header |          |                         |
+----------+----+-----+--------+--------+----------+-------------------------+

+----------+ EthernetOverhead

           +----------+ UDPOverhead

                      +--------+ Encryptor.PacketOverhead

                               +--------+ Encryptor.FrameOverhead

           +---------------------------------------+ sleeveForwarder.overheadDF

           +-----------------------------------------------------------------+ msgTooBigError.PMTU

                      +------------------------------------------------------+ sleeveForwarder.maxPayload

                                                   +-------------------------+ sleeveForwarder.effectivePMTU

(import into ASCIIFlow for serious edits)

I made this diagram to understand the PMTU discovery - I think it would make a helpful comment in sleeve.go:

I wonder if it could be made to fit in 80 columns by transposing it so that the packet structure runs from top to bottom.
More substantially, I think the EthernetOverhead bar is in the wrong place.  It should apply to the "Overlay Ethernet" box.  I don't think the outer ethernet header is ever relevant, and it can be dropped.  The outermost thing we ever care about is the PMTU reported by the kernel, and that's the packet size at the IP layer.

+----+-----+--------+--------+----------+-------------------------+
| IP | UDP | Sleeve | Sleeve | Overlay  | Overlay Layer 3 Payload |
|    |     | Packet | Frame  | Ethernet |                         |
|    |     | Header | Header |          |                         |
+----+-----+--------+--------+----------+-------------------------+

+----------+ UDPOverhead

           +--------+ Encryptor.PacketOverhead

                    +--------+ Encryptor.FrameOverhead

                             +----------+ EthernetOverhead

+---------------------------------------+ sleeveForwarder.overheadDF

+------------------------ msgTooBigError.PMTU --------------------+

           +------------- sleeveForwarder.maxPayload -------------+

          sleeveForwarder.effectivePMTU +-------------------------+

It looks like log.Debug behaves differently to log.Println in that it doesn't interpolate arguments with spaces:
 956         log.Println(fwd.logPrefix(),
 957             "Effective PMTU verified at", epmtu)

produces
<elided> Effective PMTU verified at 1438

whereas
 903     log.Debug(fwd.logPrefix(),
 904         "sendPMTUTest: epmtu candidate", fwd.epmtuCandidate)

produces
<elided> sendPMTUTest: epmtu candidate1438

This is following Go standard behaviour:

fmt.Sprint : "Spaces are added between operands when neither is a string."
fmt.Sprintln : "Spaces are always added between operands and a newline is appended."

Shouldn't MaxUDPPacketSize in router/consts.go be 65535 and not 65536? (it's wrong on master too...)

Indeed it should.  It's possible to trigger a spurious EMSGSIZE log entry by doing ping -M dont -s $[65536-28-6-14-14-20-8] across the weave network, when really it should be dropped with the same warning as for larger packets.

trigger a spurious EMSGSIZE log entry by doing ping -M dont -s $[65536-28-6-14-14-20-8] across the weave network

Nope, I just looked at the code/logs in more detail:  It actually triggers the connection to be dropped and re-established.

msgTooBigError.PMTU -> msgTooBigError.underlayPMTU

Ok

sleeveForwarder.effectivePMTU -> sleeveForwarder.overlayPMTU
sleeveForwarder.emptu* -> sleeveForwarder.opmtu*

Considering this now, one thing I notice is that the term "PMTU" in "effectivePMTU"   is only tenuously linked to PMTUs (the notion of a PMTU for the L2 overlay network doesn't make sense).  So I think we should just refer to MTU for these, not PMTU.
While I agree that the established naming is not ideal, I'm not sure I agree that we need to qualify it with "overlay".  It's not like we have a lot of mixing of overlay and underlay MTUs.  Apart from the few uses of the underlay PMTU in msgTooBigError (to be addressed as proposed), all uses refer to the overlay MTU.  And I don't think omtuCandidate is greatly less obscure than mtuCandidate.  So I think we can simply drop the "effective", and just refer to the MTU, with a comment for the fields to clarify that they refer to the overlay MTU.

So I think we can simply drop the "effective", and just refer to the MTU, with a comment for the fields to clarify that they refer to the overlay MTU.

Concur.

Looking at @awh's diagram, it suggests that we don't need all three of maxPayload, overheadDF and mtu (as overheadDF + mtu = maxPayload + UDPOverhead, with UDPOVerhead constant).  But I don't propose to clean that up now.

This is following Go standard library behaviour:
fmt.Sprint : "Spaces are added between operands when neither is a string."
fmt.Sprintln : "Spaces are always added between operands and a newline is appended."

but we're dealing with logrus, which is based around log entries, so the newline part doesn't apply.  So, there is no difference between log.Println and log.Print, except for the space insertion behaviour.  Oh, and they are exactly the same as Infoln and Info.  Obviously.
(And while I'm hating on logrus, why do the methods check the log level in logger.go when deciding whether to log an entry, and then immediately check it again in the corresponding method in entry.go?)

Ok, I've rebased and I think I've addressed all the comments that have not been discussed with some other conclusion.  I've put it the results in a separate branch to avoid orphaning comments here - https://github.com/dpw/weave/tree/fdp-prep-fixed .  All the changes in response to the discussion here are broken out to make it easy to review them.  If this meets with approval, I'll squash all the commits marked "Fix" to produce something ready for merging.
Tests currently don't pass on circle CI due to the "docker-py" breakage that affects everything.  I guess I'll try again tomorrow.

Ok, I've rebased and I think I've addressed all the comments

Thankyou - will take a look.

Tests currently don't pass on circle CI due to the "docker-py" breakage that affects everything. I guess I'll try again tomorrow.

Tests passing with Bryan's docker-py fix: https://circleci.com/gh/weaveworks/weave/2642

That seems to cover everything - a few minor comments:
+	// we can receive a heartbeat before confirmed() has set up
+	// heartbeatTimeout
+	if fwd.heartbeatTimeout != nil {
+		fwd.heartbeatTimeout.Reset(HeartbeatTimeout)
+	}
+

This would be the bug I didn't find then :smile:

I'll hoist the call to handleControlMsg up into the main forwarder loop to make the distinction clearly and for improved symmetry.

Great compromise - much improved.
+	switch newSrcMac, conflictPeer := router.Macs.Add(srcMac, router.Ourself.Peer); {
+	case newSrcMac:
+  		log.Println("Discovered local MAC", srcMac)
+
+	case conflictPeer != nil:
+		// The MAC cache has an entry for the source MAC
+		// associated with another peer.  This probably means
+		// we are seeing a frame we injected ourself.  That
+		// shouldn't happen, but discard it just in case.
+		log.Errorln("Captured frame from MAC (", srcMac, ") associated with another peer", conflictPeer)
+		return nil
 	}

This use of switch is pretty obscure - I had to scour the language spec for a few minutes to figure out what it does...

This use of switch is pretty obscure - I had to scour the language spec for a few minutes to figure out what it does...

Yes. Fun, isn't it?

Rebased and fixed squashed.

I am ashamed that I didn't manage to annoy bin/lint at all in this PR.  I will try harder in future.


Players Not in Sync/Updating

Player Position, Yaw, Pitch and Pose is not being updated. I heard someoen was already working on it so Im just putting this issue here.

Player Position, Yaw, Pitch and Pose is not being updated. I heard someoen was already working on it so Im just putting this issue here.

hmmm? then how do bows kinda work if this is the case?

@andrewgazelka, @Tebarem and me already took a look at this Issue. At the moment nothing sync related is working anymore.

@andrewgazelka, @Tebarem and me already took a look at this Issue. At the moment nothing sync related is working anymore.

hmmm let me look at this briefly seems something I can fix easily and probs was caused by me

Something merge related afaik. We already exactly know what caused it though.

Player Position, Yaw, Pitch and Pose is not being updated. I heard someoen was already working on it so Im just putting this issue here.

This cannot be right. try doing /spawn ... the entities follow

@andrewgazelka I have given you some details via discord.

I think what happened is when i added the projectile_sync in my head i added a filter for EntityKind::Arrow and it mixed with whatever was already there. Then either i removed it or someone else did and it all got mangled there.

we talked about this @Tebarem will make a video of me solving it soon

@andrewgazelka i fixed it partially i added back the old entity_state_sync now the issue is gravity doesn't seem to be applying

@andrewgazelka how are we doing physics simulation?

Looks like we arent doing any physics sim. I think that should be the next big thing we do it will sort out all of these issues

https://github.com/andrewgazelka/hyperion/pull/691 mostly fixes

recommend looking at https://www.flecs.dev/explorer

still maybe need to fix yaw for players? idk

#696 will fix it most likely

fixed in #696


mypy: Upgrade to 0.770

I was trying to work on another issue and realized the mypy version was quite old, hence this PR.
Most changes are fairly straightforward, but perhaps due to some improvements in typeshed, the input/output with Popen objects is more clearly typed, and it raises concerns over whether variables are bytes or str. I've incorporated some of these types of changes already where they seem fairly reasonable, but this approach is used quite a bit more in zephyr - and since I recall it was important to run zephyr code changes past @timabbott, I'd like to get Tim's input on that before completing this.
So: is universal_newlines applicable in general, or is it better to encode and decode via utf-8 to allow for extended characters? An explicit encoding is used at least in send_zephyr, which is what made me rethink the first approach. I think an encoding can be specified in the Popen constructor on 3.6+, but that doesn't help, particularly if the zephyr mirror might still be intended to be compatible with python2?

The commits are shown out of order above, but I've added some commits to fix a few outstanding issues that I missed when testing locally:

more type-ignores for the exec_module issue (can we re-use this code, at least in eg. botserver?)
switch the botserver into raising exceptions rather than returning them (maybe this isn't tested yet?)

These additions leave only checks failing in the zephyr mirror backend, as per above.

For the Zephyr stuff, we should just be really sure that we're not accidentally breaking it.  It's annoying to test the Zephyr code not in a production setting and I don't expect to do any further development on it in the future, so I'd prefer to minimize churn to its code over other potential goals for it.

Re type-checking zephyr, would it therefore be best to continue to check what we can for now and add ignore/Any in minimal places? Or even perhaps just exclude zephyr from the type checking, if the code isn't expected to change?

I'd probably do the Any/ignore approach if it's not super time-consuming.

@timabbott I just pushed an updated version of this, which now upgrades to mypy 0.770 (the latest)

Once this is merged, then python 3.8 testing is practical (which requires a later typed-ast than current mypy depends upon).

This is great, merged, thanks @neiljp!


Support KafkaNull in payload conversion

See https://github.com/spring-cloud/spring-cloud-stream-binder-kafka/pull/539
We need to map payloads of type KafkaNull to null when mapping method
arguments (@Payload(required = false) Foo foo.

FYI; this is already handled in SI with the NullAwarePayloadArgumentResolver - so maybe when the SPR issue is resolved, the SI resolvers can be used.


[2022 Prediction] Denis Bakhvalov - optimizing compilers

Forgive me if I'm misremembering, but I don't think this was part of the space. Would you mind moving your predictions to the next section down? Interesting predictions!

Forgive me if I'm misremembering, but I don't think this was part of the space. Would you mind moving your predictions to the next section down? Interesting predictions!

Oh, yes, you're right. Sorry about this. Will move.

Thanks!


Increment 'try' variable

To ensure that we actually probe different ports

Vijay - this should fix the infinite loop you were seeing.

I will merge this, but it's showing a Travis fail right now. Let me look into that.

Two fails in a row that look like this. I will do another Travis restart, but it looks completely unrelated.
I0227 16:58:38.607622014   13069 early_server_shutdown_finishes_inflight_calls_legacy.c:56] test_early_server_shutdown_finishes_inflight_calls/chttp2/simple_ssl_with_oauth2_fullstack
I0227 16:58:38.619195285   13070 ssl_transport_security.c:166]      HANDSHAKE START -  before/connect initialization  - UNKWN
I0227 16:58:38.619275877   13070 ssl_transport_security.c:166]                 LOOP -  before/connect initialization  - UNKWN
I0227 16:58:38.619324602   13070 ssl_transport_security.c:166]                 LOOP -     SSLv3 write client hello A  - 3WCH_A
I0227 16:58:38.619415145   13070 ssl_transport_security.c:166]      HANDSHAKE START -   before/accept initialization  - UNKWN
I0227 16:58:38.619457729   13070 ssl_transport_security.c:166]                 LOOP -   before/accept initialization  - UNKWN
I0227 16:58:38.619533347   13070 ssl_transport_security.c:166]                 LOOP -      SSLv3 read client hello A  - 3RCH_A
I0227 16:58:38.619547274   13070 ssl_transport_security.c:166]                 LOOP -     SSLv3 write server hello A  - 3WSH_A
I0227 16:58:38.619636976   13070 ssl_transport_security.c:166]                 LOOP -      SSLv3 write certificate A  - 3WSC_A
I0227 16:58:38.623372286   13070 ssl_transport_security.c:166]                 LOOP -     SSLv3 write key exchange A  - 3WSKEA
I0227 16:58:38.623385108   13070 ssl_transport_security.c:166]                 LOOP -      SSLv3 write server done A  - 3WSD_A
I0227 16:58:38.623395087   13070 ssl_transport_security.c:166]                 LOOP -               SSLv3 flush data  - 3FLUSH
I0227 16:58:38.623402776   13070 ssl_transport_security.c:166]                 LOOP - SSLv3 read client certificate   - 3RCC_A
I0227 16:58:38.623493085   13070 ssl_transport_security.c:166]                 LOOP -      SSLv3 read server hello A  - 3RSH_A
I0227 16:58:38.623862453   13070 ssl_transport_security.c:166]                 LOOP - SSLv3 read server certificate   - 3RSC_A
I0227 16:58:38.624075307   13070 ssl_transport_security.c:166]                 LOOP - SSLv3 read server key exchange  - 3RSKEA
I0227 16:58:38.624106134   13070 ssl_transport_security.c:166]                 LOOP -       SSLv3 read server done A  - 3RSD_A
I0227 16:58:38.626801316   13070 ssl_transport_security.c:166]                 LOOP - SSLv3 write client key exchang  - 3WCKEA
I0227 16:58:38.626876134   13070 ssl_transport_security.c:166]                 LOOP - SSLv3 write change cipher spec  - 3WCCSA
I0227 16:58:38.626918749   13070 ssl_transport_security.c:166]                 LOOP -         SSLv3 write finished A  - 3WFINA
I0227 16:58:38.626926387   13070 ssl_transport_security.c:166]                 LOOP -               SSLv3 flush data  - 3FLUSH
I0227 16:58:38.628339241   13070 ssl_transport_security.c:166]                 LOOP - SSLv3 read client key exchange  - 3RCKEA
I0227 16:58:38.628445644   13070 ssl_transport_security.c:166]                 LOOP -          SSLv3 read finished A  - 3RFINA
I0227 16:58:38.628496573   13070 ssl_transport_security.c:166]                 LOOP -   SSLv3 write session ticket A  - UNKWN
I0227 16:58:38.628510689   13070 ssl_transport_security.c:166]                 LOOP - SSLv3 write change cipher spec  - 3WCCSA
I0227 16:58:38.628545354   13070 ssl_transport_security.c:166]                 LOOP -         SSLv3 write finished A  - 3WFINA
I0227 16:58:38.628553847   13070 ssl_transport_security.c:166]                 LOOP -               SSLv3 flush data  - 3FLUSH
I0227 16:58:38.628572760   13070 ssl_transport_security.c:166]       HANDSHAKE DONE - SSL negotiation finished succe  - SSLOK
I0227 16:58:38.628803505   13070 ssl_transport_security.c:166]                 LOOP - SSLv3 read server session tick  - UNKWN
I0227 16:58:38.628873903   13070 ssl_transport_security.c:166]                 LOOP -          SSLv3 read finished A  - 3RFINA
I0227 16:58:38.628887346   13070 ssl_transport_security.c:166]       HANDSHAKE DONE - SSL negotiation finished succe  - SSLOK
E0227 16:58:48.629737998   13069 cq_verifier.c:324] no event received, but expected:
GRPC_FINISHED status=14 details=(null)

Working on something to remove this flakiness.
On Fri, Feb 27, 2015 at 9:07 AM Vijay Pai<EMAIL_ADDRESS>wrote:

Two fails in a row that look like this. I will do another Travis restart,
but it looks completely unrelated.
I0227 16:58:38.607622014 13069
early_server_shutdown_finishes_inflight_calls_legacy.c:56]
test_early_server_shutdown_finishes_inflight_calls/chttp2/simple_ssl_with_oauth2_fullstack
I0227 16:58:38.619195285 13070 ssl_transport_security.c:166] HANDSHAKE
START - before/connect initialization - UNKWN
I0227 16:58:38.619275877 13070 ssl_transport_security.c:166] LOOP -
before/connect initialization - UNKWN
I0227 16:58:38.619324602 13070 ssl_transport_security.c:166] LOOP - SSLv3
write client hello A - 3WCH_A
I0227 16:58:38.619415145 13070 ssl_transport_security.c:166] HANDSHAKE
START - before/accept initialization - UNKWN
I0227 16:58:38.619457729 13070 ssl_transport_security.c:166] LOOP -
before/accept initialization - UNKWN
I0227 16:58:38.619533347 13070 ssl_transport_security.c:166] LOOP - SSLv3
read client hello A - 3RCH_A
I0227 16:58:38.619547274 13070 ssl_transport_security.c:166] LOOP - SSLv3
write server hello A - 3WSH_A
I0227 16:58:38.619636976 13070 ssl_transport_security.c:166] LOOP - SSLv3
write certificate A - 3WSC_A
I0227 16:58:38.623372286 13070 ssl_transport_security.c:166] LOOP - SSLv3
write key exchange A - 3WSKEA
I0227 16:58:38.623385108 13070 ssl_transport_security.c:166] LOOP - SSLv3
write server done A - 3WSD_A
I0227 16:58:38.623395087 13070 ssl_transport_security.c:166] LOOP - SSLv3
flush data - 3FLUSH
I0227 16:58:38.623402776 13070 ssl_transport_security.c:166] LOOP - SSLv3
read client certificate - 3RCC_A
I0227 16:58:38.623493085 13070 ssl_transport_security.c:166] LOOP - SSLv3
read server hello A - 3RSH_A
I0227 16:58:38.623862453 13070 ssl_transport_security.c:166] LOOP - SSLv3
read server certificate - 3RSC_A
I0227 16:58:38.624075307 13070 ssl_transport_security.c:166] LOOP - SSLv3
read server key exchange - 3RSKEA
I0227 16:58:38.624106134 13070 ssl_transport_security.c:166] LOOP - SSLv3
read server done A - 3RSD_A
I0227 16:58:38.626801316 13070 ssl_transport_security.c:166] LOOP - SSLv3
write client key exchang - 3WCKEA
I0227 16:58:38.626876134 13070 ssl_transport_security.c:166] LOOP - SSLv3
write change cipher spec - 3WCCSA
I0227 16:58:38.626918749 13070 ssl_transport_security.c:166] LOOP - SSLv3
write finished A - 3WFINA
I0227 16:58:38.626926387 13070 ssl_transport_security.c:166] LOOP - SSLv3
flush data - 3FLUSH
I0227 16:58:38.628339241 13070 ssl_transport_security.c:166] LOOP - SSLv3
read client key exchange - 3RCKEA
I0227 16:58:38.628445644 13070 ssl_transport_security.c:166] LOOP - SSLv3
read finished A - 3RFINA
I0227 16:58:38.628496573 13070 ssl_transport_security.c:166] LOOP - SSLv3
write session ticket A - UNKWN
I0227 16:58:38.628510689 13070 ssl_transport_security.c:166] LOOP - SSLv3
write change cipher spec - 3WCCSA
I0227 16:58:38.628545354 13070 ssl_transport_security.c:166] LOOP - SSLv3
write finished A - 3WFINA
I0227 16:58:38.628553847 13070 ssl_transport_security.c:166] LOOP - SSLv3
flush data - 3FLUSH
I0227 16:58:38.628572760 13070 ssl_transport_security.c:166] HANDSHAKE
DONE - SSL negotiation finished succe - SSLOK
I0227 16:58:38.628803505 13070 ssl_transport_security.c:166] LOOP - SSLv3
read server session tick - UNKWN
I0227 16:58:38.628873903 13070 ssl_transport_security.c:166] LOOP - SSLv3
read finished A - 3RFINA
I0227 16:58:38.628887346 13070 ssl_transport_security.c:166] HANDSHAKE
DONE - SSL negotiation finished succe - SSLOK
E0227 16:58:48.629737998 13069 cq_verifier.c:324] no event received, but
expected:
GRPC_FINISHED status=14 details=(null)
—
Reply to this email directly or view it on GitHub
https://github.com/grpc/grpc/pull/872#issuecomment-76430457.

New flakiness!
I0227 17:14:05.544941586   12031 writes_done_hangs_with_pending_read_legacy.c:56] test_writes_done_hangs_with_pending_read/chttp2/fake_secure_fullstack
I0227 17:14:09.543051158   12032 fake_transport_security.c:395] Client prepared CLIENT_INIT.
I0227 17:14:09.543179640   12032 fake_transport_security.c:439] Server received CLIENT_INIT.
I0227 17:14:09.543327871   12032 fake_transport_security.c:395] Server prepared SERVER_INIT.
I0227 17:14:09.543366948   12032 fake_transport_security.c:439] Client received SERVER_INIT.
I0227 17:14:09.543377973   12032 fake_transport_security.c:395] Client prepared CLIENT_FINISHED.
I0227 17:14:09.543405480   12032 fake_transport_security.c:439] Server received CLIENT_FINISHED.
I0227 17:14:09.543415042   12032 fake_transport_security.c:395] Server prepared SERVER_FINISHED.
I0227 17:14:09.543423362   12032 fake_transport_security.c:404] Server is done.
I0227 17:14:09.543679329   12032 fake_transport_security.c:439] Client received SERVER_FINISHED.
I0227 17:14:09.543690991   12032 fake_transport_security.c:445] Client is done.
E0227 17:14:10.545097805   12031 cq_verifier.c:205] assertion failed: e->data.finished.status == ev->data.finished.status
FAILED: bins/dbg/chttp2_fake_security_writes_done_hangs_with_pending_read_legacy_test [ret=-6]

Ok.
Passes.
Significantly deflakes the C core.
Merge away?


dest.c: Use monotonic time for cups_enum_dest

Patch written by Adam Williamson.
gettimeofday() is vulnerable to clock jumps, so it is better to use clock_gettime() to avoid issues with libcups if system changes clock, f.e. in Live CDs.

[master 0350eba54] Add cupsGetClock API.
Also in libcups repository.

To no avail... here is the post I found about Mac and CLOCK_MONOTONIC https://discussions.apple.com/thread/253778121?page=2&sortBy=rank .

The CI failure might be cause by load on the system or virtualization speed - the testclock program is just seeing that the monotonic clock is increasing and roughly close to the expected (wall) clock time.  Having jitter that large isn't ideal but as long as the values are increasing it won't cause a problem.

@michaelrsweet so I would ifdef the test binary and raise acceptable delta for Mac? Or introduce a new function which will allow us to set acceptable delta and use that with a different value on Mac?

@michaelrsweet so I would ifdef the test binary and raise acceptable delta for Mac? Or introduce a new function which will allow us to set acceptable delta and use that with a different value on Mac?

This is just in the unit test program, so we can increase the acceptable delta (say to 1 second) or just drop it to avoid future woe. The key is to flag values going backwards.


Error while using https

When I try something like that:
paperless = Paperless(
    "https://servername/paperless/api/",
    "xxxxx",
    request_opts={"ssl": True}
)

I get the error
aiohttp.client_exceptions.ClientConnectorError: Cannot connect to host https:80 ssl:True [Name or service not known]
ERROR    asyncio - Unclosed client session
client_session: <aiohttp.client.ClientSession object at 0x7f58441fc210>

I wonder
(a) why port 80 is used and
(b) how I can use a SSLContext or directly cacert.pem.

Hi, thanks for your issue.
Please initialize the Paperless object by just passing the servername/paperless part of your URL, pypaperless will create the full api path on its own: http://{self.host}/api/{path}". And yes, there is a relic from the beginning of the project, which I will fix tomorrow.
Is your Paperless-ngx behind a reverse proxy which enforces https and a sub-path? Please tell me what happens if you request the following urls in your browser.

https://servername/paperless/api/
http://servername/paperless/api/

It should respond with 200 OK on both requests and that json object:

You may pass any option via request_opts, which an aiohttp.ClientSession allows for its request method: self._session.request(method, url, **kwargs). I will update the documentation to be more clear about it.

Ran into this as well. I would like to use the https endpoint as http is firewalled on my paperless instance.

Hi, yes, my paperless-ngx is behind a reverse proxy which enforces https and a sub-path.
I have to use the https protocol prefix in the host variable, because otherwise the API url would be used with the http protocol as you pointed out. Your both requests
https://servername/paperless/api/
http://servername/paperless/api/

lead to the same error message I mentioned above. Furthermore, if I use
https://servername:443/paperless/api/

the result is the same ("cannot connect to host https:80").

For me there was some paperless configuration missing to get the correct api urls back:
PAPERLESS_USE_X_FORWARD_HOST=1
PAPERLESS_USE_X_FORWARD_PORT=1
PAPERLESS_PROXY_SSL_HEADER=["HTTP_X_FORWARDED_PROTO", "https"]

This was because of a reverse proxy terminating the ssl connection and django ignoring the specific headers for that. But getting the api urls from the api root path still didn't work because of the hardcoded protocol here. Patching that to https worked for me as a workaround, but it would be better if that could be configured.
The patched version allows me to use Paperless("hostname:443", ..., request_opts={ "ssl": False }) where request_opts={ "ssl": False } disables the cert check but still allows for ssl if there is https in the url.

I'm using the same paperless reverse proxy configuration as @hapm does, my development paperless container is simply http.
Whatever, I gonna make use of yarl to parse all necessary url parts, as it's already a sub dependency. Goal is letting developers pass either a single hostname, or a complete url path to the Paperless object (especially when using sub paths as @tzepter mentioned above). It will parse everything it needs itself from the given infos. When just passing a hostname, the Paperless object initiates a basic discovery task for checking if https is available, https>http. I don't want to over engineer things here, because it's just a rest wrapper and users of it are clever enough to handle urls right.

Hm, I understood you - @tb1337 - to mean that if I extend the paperless environment with the variables shown by @hapm, pypaperless should work if I use a complete URL path. This is not the case: I still get the error message
Cannot connect to host https:80 ssl:False [Name or service not known]
At the same time, however, this works without any problem:
import httpx
host = "https://server/paperless"
token = "xxxx"
resp = httpx.post(host + "/api/documents/post_document/",
                      headers={"Authorisation": "Token " + token},
                      files={'document': ('test.pdf', open('./test.pdf', 'rb'), 'application/pdf')},
                      data={"title": "My first upload", "created": "2023-12-21"},
                      verify='./cacert.pem')
    if resp.status_code == 200:
          pass

Why does aiohttp think to use port 80?

It does that because I implemented a bug, as mentioned above. Currently fixing it. ;)

In order to request via http, one has to explicitly instantiate the Paperless object this way: paperless = Paperless("http://hostname").
You have to instantiate it so: paperless = Paperless("https://hostname/paperless"), Paperless("hostname/paperless") will work too (defaults to https).
I'll push the branch soon, could you test with the branch if the solution is working for you now? I tested it locally against my dev server (http) and prod server (https), it is working for me. Before I can release a new version, I have to write docs and solve another issue. I'm busy this weekend, so we have to wait until Monday for that.

it seem to work. I am saying this so carefully because there was a error, but it is very likely due to another reason. To verify that I used the file under example/usage.py and fit it to my environment:
import asyncio
import logging
import os.path
import sys
import ssl
from pypaperless import Paperless

sys.path.append(os.path.abspath(os.path.join(os.path.dirname(__file__), os.path.pardir)))

logging.basicConfig(
    level=logging.DEBUG,
    format="%(levelname)-8s %(name)s - %(message)s",
)

from pypaperless import Paperless  # noqa

context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
context.load_verify_locations('./cacert.pem')
context.verify_mode = ssl.CERT_REQUIRED
context.check_hostname = True

paperless = Paperless(
    "https://server/paperless/",
    "xxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
    request_opts={"ssl_context": context}
)

async def main():
    """Execute main function."""
    async with paperless as p:
        correspondents = {}
        async for item in p.correspondents.iterate():
            correspondents[item.id] = item

        documents = await p.documents.get(page=1)
        for item in documents.items:
            print(
                f"Correspondent of document {item.id} is: {correspondents[item.correspondent].name}!"  # noqa
            )
            await asyncio.sleep(0.25)

if __name__ == "__main__":
    asyncio.run(main())

The result is:
DEBUG    asyncio - Using selector: EpollSelector
INFO     pypaperless[servername] - Fetching api endpoints.
DEBUG    pypaperless[servername] - Json-Request GET (200): https://servername/paperless/api/
DEBUG    pypaperless[servername] - Unused endpoints: logs
INFO     pypaperless[servername] - Initialized.
DEBUG    pypaperless[servername] - Json-Request GET (200): https://servername/paperless/api/correspondents/?page=1&page_size=50
DEBUG    pypaperless[servername] - Json-Request GET (200): https://histton/servername/api/documents/?page=1&page_size=100
Correspondent of document 18 is: XXX!
Correspondent of document 15 is: XXX!
Correspondent of document 21 is: XXXXXXXXXXX!
INFO     pypaperless[servername] - Closed.
Traceback (most recent call last):
  File "/home/user/Dokumente/code/python/EloMigration/example.py", line 50, in <module>
    asyncio.run(main())
  File "/usr/lib/python3.11/asyncio/runners.py", line 190, in run
    return runner.run(main)
           ^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/asyncio/runners.py", line 118, in run
    return self._loop.run_until_complete(task)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/asyncio/base_events.py", line 653, in run_until_complete
    return future.result()
           ^^^^^^^^^^^^^^^
  File "/home/user/Dokumente/code/python/EloMigration/example.py", line 36, in main
    async with paperless as p:
  File "/home/user/Dokumente/code/python/EloMigration/env/lib/python3.11/site-packages/pypaperless/__init__.py", line 276, in __aexit__
    raise exc
  File "/home/user/Dokumente/code/python/EloMigration/example.py", line 44, in main
    f"Correspondent of document {item.id} is: {correspondents[item.correspondent].name}!"  # noqa
                                               ~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^
KeyError: None

If I used request_opts={"ssl": False} the result is the same.

Yes. It seems like one of your documents doesn’t have a correspondent. None raises the KeyError and there we are. Am I right?

You are right. I just exercise document migration now. ;)
Thx!

Thanks for your issue guys. I will merge until Monday