Skip to content

[swift v1]: introduce a TempURLKey argument for objects.CreateTempURLOpts struct#2602

Merged
mandre merged 1 commit intogophercloud:masterfrom
kayrus:custom-tempurl-key
Mar 31, 2023
Merged

[swift v1]: introduce a TempURLKey argument for objects.CreateTempURLOpts struct#2602
mandre merged 1 commit intogophercloud:masterfrom
kayrus:custom-tempurl-key

Conversation

@kayrus
Copy link
Copy Markdown
Contributor

@kayrus kayrus commented Mar 30, 2023

Fixes #2601

@coveralls
Copy link
Copy Markdown

coveralls commented Mar 30, 2023 •
edited
Loading

Coverage Status

Coverage: 79.076% (-0.05%) from 79.124% when pulling 330f72e on kayrus:custom-tempurl-key into 7220520 on gophercloud:master.

@kayrus kayrus force-pushed the custom-tempurl-key branch 2 times, most recently from a4c6d6d to fcb1ed1 Compare March 30, 2023 10:09
@kayrus
Copy link
Copy Markdown
Contributor Author

kayrus commented Mar 30, 2023 •
edited
Loading

@EmilienM @pierreprinetti looks like something is wrong with the CI

@mandre
Copy link
Copy Markdown
Contributor

mandre commented Mar 30, 2023

Master failed with an unrelated error downloading the cirros image during devstack deployment:

+ functions:upload_image:142               :   wget --progress=dot:giga -c http://download.cirros-cloud.net/0.5.2/cirros-0.5.2-x86_64-disk.img -O /home/runner/work/gophercloud/gophercloud/devstack/files/cirros-0.5.2-x86_64-disk.img
--2023-03-30 10:39:29--  http://download.cirros-cloud.net/0.5.2/cirros-0.5.2-x86_64-disk.img
Resolving download.cirros-cloud.net (download.cirros-cloud.net)... 64.90.42.85, 2607:f298:6:a036::bd6:a72a
Connecting to download.cirros-cloud.net (download.cirros-cloud.net)|64.90.42.85|:80... connected.
HTTP request sent, awaiting response... 429 Too Many Requests
2023-03-30 10:39:30 ERROR 429: Too Many Requests.

However, the other jobs failed for a legitimate reason:

 === RUN   TestObjects
    objects_test.go:39: Create object headers: &{ContentLength:0 ContentType:text/html; charset=UTF-8 Date:2023-03-30 10:46:27 +0000 GMT TransID:tx505c20822268478db237c-0064256883}
    convenience.go:36: Failure in objects_test.go, line 96: unexpected error "Unable to obtain the Temp URL key."
--- FAIL: TestObjects (0.53s)

@kayrus
Copy link
Copy Markdown
Contributor Author

kayrus commented Mar 30, 2023

However, the other jobs failed for a legitimate reason:

@mandre I forgot about the scrolling, have seen only master. Interesting, I was sure that links generated with an empty key shouldn't work. Let me check in my env.

@kayrus
Copy link
Copy Markdown
Contributor Author

kayrus commented Mar 30, 2023 •
edited
Loading

@mandre funny, it looks like tempurl acceptance tests have never worked.

@kayrus kayrus force-pushed the custom-tempurl-key branch 6 times, most recently from e5756e3 to 96c2522 Compare March 30, 2023 18:55
@kayrus kayrus force-pushed the custom-tempurl-key branch from 96c2522 to 330f72e Compare March 30, 2023 18:59
@kayrus kayrus mentioned this pull request Mar 30, 2023
Open
@kayrus
Copy link
Copy Markdown
Contributor Author

kayrus commented Mar 30, 2023

@mandre ready to review.

@kayrus
Copy link
Copy Markdown
Contributor Author

kayrus commented Mar 31, 2023

@EmilienM @pierreprinetti @mandre kindly ping.

@mandre mandre added this to the v1.4.0 milestone Mar 31, 2023
mandre
mandre reviewed Mar 31, 2023
View reviewed changes
acceptance/openstack/objectstorage/v1/objects_test.go
th.AssertNoErr(t, err)

resp, err := http.Get(objURLs[i])
resp, err := client.ProviderClient.HTTPClient.Get(objURLs[i])
Copy link
Copy Markdown
Contributor

@mandre mandre Mar 31, 2023

Choose a reason for hiding this comment

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

Shouldn't we keep using an unauthenticated client for downloading the objects using the temp URL?

Copy link
Copy Markdown
Contributor Author

@kayrus kayrus Mar 31, 2023

Choose a reason for hiding this comment

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

it's already unauthenticated. if you take a look on the debug code you can see that there are no headers used. this is just more convenient way to reuse the existing http client on the low level + get a bonus for http request/response debug.

mandre
mandre approved these changes Mar 31, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@mandre mandre left a comment

Choose a reason for hiding this comment

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

LGTM

@mandre mandre merged commit 6e0b5ea into gophercloud:master Mar 31, 2023
@kayrus kayrus deleted the custom-tempurl-key branch March 31, 2023 14:10
@kayrus kayrus mentioned this pull request Sep 20, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mandre mandre mandre approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v1.4.0

Development

Successfully merging this pull request may close these issues.

[swift v1]: add an ability to specify a custom tempurl key

3 participants

@kayrus @coveralls @mandre