Skip to content
This repository has been archived by the owner on Aug 2, 2021. It is now read-only.

cmd/swarm: remove --no-track, add --progress flag #1850

Merged
merged 2 commits into from
Oct 2, 2019
Merged

Conversation

nonsense
Copy link
Contributor

@nonsense nonsense commented Oct 2, 2019

If the API pointed to by swarm up doesn't support pushsync and the user hasn't added --no-track, we currently get an error:

[lash@furioso tmp]$ swarm --bzzapi https://swarm-gateways.net up photo_2019-10-02_10-10-59.jpg 
Fatal: failed to get tag data for hash: unexpected HTTP status: 404 Not Found

Even with session affinity, the progress bar doesn't work with pullsync (default Swarm configuration) today.

This PR is fixing this problem by falling back to swarm up functionality, which just displays the hash of the upload. If the user knows that the API supports pushsync, they can use --progress to see the progress bar.

@skylenet skylenet added this to the 0.5.1 milestone Oct 2, 2019
@skylenet skylenet self-requested a review October 2, 2019 10:02
@skylenet
Copy link
Contributor

skylenet commented Oct 2, 2019

The 404 is because the swarm-gateways.net requests are load balanced. It uses a cookie to persist the session. If you don't reuse that cookie on the upcoming requests, you'll end up on another node, that doesn't know about the tag.

@nonsense nonsense requested a review from janos October 2, 2019 10:13
@nonsense nonsense changed the title cmd/swarm: fix swarm up to work with push and pull sync APIs cmd/swarm: remove --no-track, add --progress flag Oct 2, 2019
@janos
Copy link
Member

janos commented Oct 2, 2019

With #1851 and bac377d uploads on public gateway work fine without changes to cli options.

@nonsense
Copy link
Contributor Author

nonsense commented Oct 2, 2019

@janos we also found a bug with the progress bar and an encrypted upload.

I think the new swarm up functionality is not stable enough, so that we have it as a default, I'd rather we enable it behind a flag, similar to the --sync-mode=push we have right now, until it is stable.

@acud
Copy link
Member

acud commented Oct 2, 2019

@nonsense you'll need to remove the --no-track flags i added across tests in order to generate machine readable output (see travis)

Copy link
Member

@janos janos left a comment

Choose a reason for hiding this comment

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

This is the best option that we can do now.

@nonsense
Copy link
Contributor Author

nonsense commented Oct 2, 2019

@janos @acud seeing that all tests were modified with --no-track should have triggered a warning for us to check the default behaviour of swarm up without additional flags :)

@nonsense nonsense merged commit 3cf5607 into master Oct 2, 2019
@acud acud deleted the fix-swarm-up branch October 3, 2019 04:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants