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

Hotfix capture with empty UUID #70

Closed
wants to merge 9 commits into from
Closed

Hotfix capture with empty UUID #70

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
4b91893
specify uuid
Aug 14, 2024
9ce2afa
tidy
Aug 14, 2024
7f12943
fix
Aug 14, 2024
5893386
rename package
Aug 14, 2024
4fabcef
rollback
Aug 14, 2024
101b879
rollback all
Aug 19, 2024
c5dea82
Merge remote-tracking branch 'origin/master'
Aug 19, 2024
51a7f5c
Merge branch 'PostHog:master' into master
evgeek Aug 23, 2024
46fabd4
init
Aug 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions feature_flags_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ func TestFlagGroup(t *testing.T) {
w.Write([]byte(fixture("test-decide-v3.json")))
} else if strings.HasPrefix(r.URL.Path, "/api/feature_flag/local_evaluation") {
w.Write([]byte(fixture("feature_flag/test-flag-group-properties.json")))
} else if strings.HasPrefix(r.URL.Path, "/batch/") {
} else if strings.HasPrefix(r.URL.Path, "/capture/") {
// Ignore batch requests
} else {
t.Error("Unknown request made by library")
Expand Down Expand Up @@ -4436,7 +4436,7 @@ func TestFlagWithTimeoutExceeded(t *testing.T) {
w.Write([]byte(fixture("test-decide-v3.json")))
} else if strings.HasPrefix(r.URL.Path, "/api/feature_flag/local_evaluation") {
w.Write([]byte(fixture("feature_flag/test-flag-group-properties.json")))
} else if strings.HasPrefix(r.URL.Path, "/batch/") {
} else if strings.HasPrefix(r.URL.Path, "/capture/") {
// Ignore batch requests
} else {
t.Error("Unknown request made by library")
Expand Down Expand Up @@ -4538,7 +4538,7 @@ func TestFlagDefinitionsWithTimeoutExceeded(t *testing.T) {
} else if strings.HasPrefix(r.URL.Path, "/api/feature_flag/local_evaluation") {
time.Sleep(11 * time.Second)
w.Write([]byte(fixture("feature_flag/test-flag-group-properties.json")))
} else if strings.HasPrefix(r.URL.Path, "/batch/") {
} else if strings.HasPrefix(r.URL.Path, "/capture/") {
// Ignore batch requests
} else {
t.Error("Unknown request made by library")
Expand Down
2 changes: 1 addition & 1 deletion message.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func makeTimestamp(t time.Time, def time.Time) time.Time {
return t
}

// This structure represents objects sent to the /batch/ endpoint. We don't
// This structure represents objects sent to the /capture/ endpoint. We don't
// export this type because it's only meant to be used internally to send groups
// of messages in one API call.
type batch struct {
Expand Down
2 changes: 1 addition & 1 deletion posthog.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,7 @@ func (c *client) send(msgs []message) {

// Upload serialized batch message.
func (c *client) upload(b []byte) error {
url := c.Endpoint + "/batch/"
url := c.Endpoint + "/capture/"
Copy link
Contributor

Choose a reason for hiding this comment

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

wait, I don't understand; you're removing the /batch functionality? I don't think that's a good long-term fix for this; folks are using that piece, too. If we're looking to fix something quickly, we should just revert the change and then fix it more permanently instead of continuing to change more things.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is just a quick (not good) fix, i describe my thoughts here - #68 (comment).

Do you think you can configure /batch on the server side to work with an empty uuid, like /capture? If (I hope) this works out, previous PR will be ok. If not, we will need to think about how best to implement the functionality from the SDK side.

But for now you're right - rollback is better. I don't know all the SDK use cases and can't be sure it won't break something else.
Rollback PR - #71

req, err := http.NewRequest("POST", url, bytes.NewReader(b))
if err != nil {
c.Errorf("creating request - %s", err)
Expand Down
10 changes: 5 additions & 5 deletions posthog_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -911,7 +911,7 @@ func TestGetFeatureFlagPayloadWithNoPersonalApiKey(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if strings.HasPrefix(r.URL.Path, "/decide") {
w.Write([]byte(fixture("test-decide-v3.json")))
} else if !strings.HasPrefix(r.URL.Path, "/batch") {
} else if !strings.HasPrefix(r.URL.Path, "/capture") {
t.Errorf("client called an endpoint it shouldn't have: %s", r.URL.Path)
}
}))
Expand Down Expand Up @@ -1097,7 +1097,7 @@ func TestGetFeatureFlagWithNoPersonalApiKey(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if strings.HasPrefix(r.URL.Path, "/decide") {
w.Write([]byte(fixture("test-decide-v3.json")))
} else if !strings.HasPrefix(r.URL.Path, "/batch") {
} else if !strings.HasPrefix(r.URL.Path, "/capture") {
t.Errorf("client called an endpoint it shouldn't have: %s", r.URL.Path)
}
}))
Expand Down Expand Up @@ -1543,7 +1543,7 @@ func TestComplexFlag(t *testing.T) {
w.Write([]byte(fixture("test-decide-v3.json")))
} else if strings.HasPrefix(r.URL.Path, "/api/feature_flag/local_evaluation") {
w.Write([]byte(fixture("test-api-feature-flag.json")))
} else if !strings.HasPrefix(r.URL.Path, "/batch") {
} else if !strings.HasPrefix(r.URL.Path, "/capture") {
t.Errorf("client called an endpoint it shouldn't have")
}
}))
Expand Down Expand Up @@ -1595,7 +1595,7 @@ func TestMultiVariateFlag(t *testing.T) {
w.Write([]byte(fixture("test-decide-v3.json")))
} else if strings.HasPrefix(r.URL.Path, "/api/feature_flag/local_evaluation") {
w.Write([]byte("{}"))
} else if !strings.HasPrefix(r.URL.Path, "/batch") {
} else if !strings.HasPrefix(r.URL.Path, "/capture") {
t.Errorf("client called an endpoint it shouldn't have")
}
}))
Expand Down Expand Up @@ -1647,7 +1647,7 @@ func TestDisabledFlag(t *testing.T) {
w.Write([]byte(fixture("test-decide-v3.json")))
} else if strings.HasPrefix(r.URL.Path, "/api/feature_flag/local_evaluation") {
w.Write([]byte("{}"))
} else if !strings.HasPrefix(r.URL.Path, "/batch") {
} else if !strings.HasPrefix(r.URL.Path, "/capture") {
t.Errorf("client called an endpoint it shouldn't have")
}
}))
Expand Down
Loading